[PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[James Chapman]

[-- Attachment #1: Type: text/plain, Size: 84 bytes --]

Add DS1337 RTC chip driver.

Signed-off-by: James Chapman <jchapman@katalix.com>




[-- Attachment #2: ds1337.patch --]
[-- Type: text/plain, Size: 11211 bytes --]

diff -Nru a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
--- a/drivers/i2c/chips/Kconfig	2005-02-27 20:42:22 +00:00
+++ b/drivers/i2c/chips/Kconfig	2005-02-27 20:42:22 +00:00
@@ -62,6 +62,17 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called asb100.
 
+config SENSORS_DS1337
+      	tristate "Dallas Semiconductor DS1337 Real Time Clock"
+	depends on I2C && EXPERIMENTAL
+	select I2C_SENSOR
+	help
+	  If you say yes here you get support for Dallas Semiconductor
+	  DS1337 real-time clock chips. 
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ds1337.
+
 config SENSORS_DS1621
       	tristate "Dallas Semiconductor DS1621 and DS1625"
 	depends on I2C && EXPERIMENTAL
diff -Nru a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
--- a/drivers/i2c/chips/Makefile	2005-02-27 20:42:22 +00:00
+++ b/drivers/i2c/chips/Makefile	2005-02-27 20:42:22 +00:00
@@ -11,6 +11,7 @@
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
+obj-$(CONFIG_SENSORS_DS1337)	+= ds1337.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o
 obj-$(CONFIG_SENSORS_FSCHER)	+= fscher.o
diff -Nru a/drivers/i2c/chips/ds1337.c b/drivers/i2c/chips/ds1337.c
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/drivers/i2c/chips/ds1337.c	2005-02-27 20:42:22 +00:00
@@ -0,0 +1,394 @@
+/*
+ *  linux/drivers/i2c/chips/ds1337.c
+ *
+ *  Copyright (C) 2005 James Chapman <jchapman@katalix.com>
+ *
+ *	based on linux/drivers/acron/char/pcf8583.c
+ *  Copyright (C) 2000 Russell King
+ *
+ * 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.
+ *
+ * Driver for Dallas Semiconductor DS1337 real time clock chip
+ */
+
+/* Define to compile in pr_debug() trace */
+#undef DEBUG
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sensor.h>
+#include <linux/string.h>
+#include <linux/rtc.h>		/* get the user-level API */
+#include <linux/bcd.h>
+#include <linux/list.h>
+
+#define DS1337_NUM_REGS		16
+
+/* FIXME - how do we export these interface constants? */
+#define DS1337_GET_DATE		0
+#define DS1337_SET_DATE		1
+
+static int debug = 0;
+module_param(debug, int, S_IRUGO | S_IWUSR);
+
+/*
+ * Functions declaration
+ */
+static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+SENSORS_INSMOD_1(ds1337);
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter);
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
+static void ds1337_init_client(struct i2c_client *client);
+static int ds1337_detach_client(struct i2c_client *client);
+static int ds1337_command(struct i2c_client *client, unsigned int cmd, void *arg);
+
+/*
+ * Driver data (common to all clients)
+ */
+static struct i2c_driver ds1337_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "ds1337",
+	.id		= I2C_DRIVERID_DS1337,
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= ds1337_attach_adapter,
+	.detach_client	= ds1337_detach_client,
+	.command	= ds1337_command,
+};
+
+/*
+ * Client data (each client gets its own)
+ */
+struct ds1337_data {
+	struct i2c_client client;
+	struct list_head list;
+	int id;
+};
+
+/*
+ * Internal variables
+ */
+static int ds1337_id = 0;
+static LIST_HEAD(ds1337_clients);
+
+static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
+{
+	s32 tmp = i2c_smbus_read_byte_data(client, reg);
+
+	if (tmp < 0)
+		return -EIO;
+
+	*value = (u8) (tmp & 0xff);
+
+	return 0;
+}
+
+/*
+ * Chip access functions
+ */
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+	int result;
+	u8 buf[7];
+	u8 val;
+	struct i2c_msg msg[2];
+	u8 offs = 0;
+
+	if (debug >= 1)
+		pr_debug("%s: client=%p, dt=%p\n", __FUNCTION__, client, dt);
+
+	if (!dt || !client)
+		return -EINVAL;
+
+	memset(buf, 0, sizeof(buf));
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = 1;
+	msg[0].buf = &offs;
+
+	msg[1].addr = client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].len = sizeof(buf);
+	msg[1].buf = &buf[0];
+
+	result = client->adapter->algo->master_xfer(client->adapter,
+						    &msg[0],
+						    2);
+
+	if (debug >= 2)
+		pr_debug("%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
+			 __FUNCTION__, result, buf[0], buf[1], buf[2], buf[3],
+			 buf[4], buf[5], buf[6]);
+
+	if (result >= 0) {
+		dt->tm_sec = BCD_TO_BIN(buf[0]);
+		dt->tm_min = BCD_TO_BIN(buf[1]);
+		val = buf[2] & 0x3f;
+		dt->tm_hour = BCD_TO_BIN(val);
+		dt->tm_wday = BCD_TO_BIN(buf[3]) - 1;
+		dt->tm_mday = BCD_TO_BIN(buf[4]);
+		val = buf[5] & 0x7f;
+		dt->tm_mon = BCD_TO_BIN(val);
+		dt->tm_year = 1900 + BCD_TO_BIN(buf[6]);
+		if (buf[5] & 0x80)
+			dt->tm_year += 100;
+
+		if (debug >= 2)
+			pr_debug("%s: secs=%d, mins=%d, "
+				 "hours=%d, mday=%d, "
+				 "mon=%d, year=%d, wday=%d\n",
+				 __FUNCTION__,
+				 dt->tm_sec, dt->tm_min,
+				 dt->tm_hour, dt->tm_mday,
+				 dt->tm_mon, dt->tm_year, dt->tm_wday);
+	} else {
+		printk(KERN_ERR "ds1337[%d]: error reading data! %d\n",
+		       client->id, result);
+		result = -EIO;
+	}
+
+	return result;
+}
+
+static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+	int result;
+	u8 buf[8];
+	u8 val;
+	struct i2c_msg msg[1];
+
+	if (debug >= 1)
+		pr_debug("%s: client=%p, dt=%p\n", __FUNCTION__, client, dt);
+
+	if (!dt || !client)
+		return -EINVAL;
+
+	if (debug >= 2)
+		pr_debug("%s: secs=%d, mins=%d, hours=%d, "
+			 "mday=%d, mon=%d, year=%d, wday=%d\n",
+			 __FUNCTION__,
+			 dt->tm_sec, dt->tm_min, dt->tm_hour,
+			 dt->tm_mday, dt->tm_mon, dt->tm_year, dt->tm_wday);
+
+	buf[0] = 0;		/* reg offset */
+	buf[1] = BIN_TO_BCD(dt->tm_sec);
+	buf[2] = BIN_TO_BCD(dt->tm_min);
+	buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
+	buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
+	buf[5] = BIN_TO_BCD(dt->tm_mday);
+	buf[6] = BIN_TO_BCD(dt->tm_mon);
+	if (dt->tm_year >= 2000) {
+		val = dt->tm_year - 2000;
+		buf[6] |= (1 << 7);
+	} else {
+		val = dt->tm_year - 1900;
+	}
+	buf[7] = BIN_TO_BCD(val);
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = sizeof(buf);
+	msg[0].buf = &buf[0];
+
+	result = client->adapter->algo->master_xfer(client->adapter,
+						    &msg[0], 1);
+	if (result < 0) {
+		printk(KERN_ERR "ds1337[%d]: error writing data! %d\n",
+		       client->id, result);
+		result = -EIO;
+	} else {
+		result = 0;
+	}
+
+	return result;
+}
+
+static int ds1337_command(struct i2c_client *client, unsigned int cmd,
+			  void *arg)
+{
+	if (debug >= 1)
+		pr_debug("%s: cmd=%d\n", __FUNCTION__, cmd);
+
+	switch (cmd) {
+	case DS1337_GET_DATE:
+		return ds1337_get_datetime(client, arg);
+
+	case DS1337_SET_DATE:
+		return ds1337_set_datetime(client, arg);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Public API for access to specific device. Useful for low-level
+ * RTC access from kernel code.
+ */
+int ds1337_do_command(int id, int cmd, void *arg)
+{
+	struct list_head *walk;
+	struct list_head *tmp;
+	struct ds1337_data *data;
+
+	list_for_each_safe(walk, tmp, &ds1337_clients) {
+		data = list_entry(walk, struct ds1337_data, list);
+		if (data->id == id) {
+			return ds1337_command(&data->client, cmd, arg);
+		}
+	}
+
+	return -ENODEV;
+}
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter)
+{
+	if (!(adapter->class & I2C_CLASS_HWMON))
+		return 0;
+	return i2c_detect(adapter, &addr_data, ds1337_detect);
+}
+
+/*
+ * The following function does more than just detection. If detection
+ * succeeds, it also registers the new chip.
+ */
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct ds1337_data *data;
+	int err = 0;
+	const char *name = "";
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		goto exit;
+
+	if (!(data = kmalloc(sizeof(struct ds1337_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct ds1337_data));
+	INIT_LIST_HEAD(&data->list);
+
+	err = -ENODEV;
+
+	/* The common I2C client data is placed right before the
+	   DS1337-specific data. */
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &ds1337_driver;
+	new_client->flags = 0;
+
+	/*
+	 * Now we do the remaining detection. A negative kind means that
+	 * the driver was loaded with no force parameter (default), so we
+	 * must both detect and identify the chip. A zero kind means that
+	 * the driver was loaded with the force parameter, the detection
+	 * step shall be skipped. A positive kind means that the driver
+	 * was loaded with the force parameter and a given kind of chip is
+	 * requested, so both the detection and the identification steps
+	 * are skipped.
+	 */
+
+	/* Default to an DS1337 if forced */
+	if (kind == 0)
+		kind = ds1337;
+
+	if (kind < 0) {		/* detection and identification */
+
+		u8 buf[DS1337_NUM_REGS];
+		int reg;
+
+		/* Check that all DS1337 registers are present */
+		for (reg = 0; reg < DS1337_NUM_REGS; reg++)
+			if (ds1337_read(new_client, reg, &buf[reg]) < 0)
+				goto exit_free;
+
+		/* Check that control register bits 5-6 are zero */
+		if (buf[14] & ((1 << 5) | (1 << 6)))
+			goto exit_free;
+
+		/* Check that status register bits 2-6 are zero */
+		if (buf[15] & ((1 << 2) | (1 << 3) | (1 << 4) |
+			       (1 << 5) | (1 << 6)))
+			goto exit_free;
+
+		kind = ds1337;
+	}
+
+	if (kind == ds1337)
+		name = "ds1337";
+
+	/* We can fill in the remaining client fields */
+	strlcpy(new_client->name, name, I2C_NAME_SIZE);
+	new_client->id = ds1337_id++;
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_free;
+
+	/* Initialize the DS1337 chip */
+	ds1337_init_client(new_client);
+
+	/* Add client to local list */
+	data->id = new_client->id;
+	list_add(&data->list, &ds1337_clients);
+
+	return 0;
+
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static void ds1337_init_client(struct i2c_client *client)
+{
+	u8 val;
+
+	/* Ensure that device is set in 24-hour mode */
+	val = i2c_smbus_read_byte_data(client, 2);
+	i2c_smbus_write_byte_data(client, 2, val | (1 << 6));
+}
+
+static int ds1337_detach_client(struct i2c_client *client)
+{
+	int err;
+	struct ds1337_data *data = i2c_get_clientdata(client);
+
+	if ((err = i2c_detach_client(client))) {
+		dev_err(&client->dev, "Client deregistration failed, "
+			"client not detached.\n");
+		return err;
+	}
+
+	list_del(&data->list);
+	kfree(data);
+	return 0;
+}
+
+static int __init ds1337_init(void)
+{
+	return i2c_add_driver(&ds1337_driver);
+}
+
+static void __exit ds1337_exit(void)
+{
+	i2c_del_driver(&ds1337_driver);
+}
+
+MODULE_AUTHOR("James Chapman <jchapman@katalix.com>");
+MODULE_DESCRIPTION("DS1337 RTC driver");
+MODULE_LICENSE("GPL");
+
+module_init(ds1337_init);
+module_exit(ds1337_exit);

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[Greg KH]

On Mon, Feb 28, 2005 at 05:14:25PM +0000, James Chapman wrote:
> +/* Define to compile in pr_debug() trace */
> +#undef DEBUG

Not needed, we do this in the makefile now.

> +	if (debug >= 1)
> +		pr_debug("%s: client=%p, dt=%p\n", __FUNCTION__, client, dt);

Please use the dev_dbg(), dev_err() and friend functions instead of
pr_debug().  It provides a sane user interface that all of the other
drivers use.

thanks,

greg k-h

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[James Chapman]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

Revised ds1337 chip driver patch.

Signed-off-by: James Chapman <jchapman@katalix.com>

- change all driver log messages to use dev_dbg() or dev_err()
- remove debug module parameter

Greg KH wrote:

> On Mon, Feb 28, 2005 at 05:14:25PM +0000, James Chapman wrote:
> 
>>+/* Define to compile in pr_debug() trace */
>>+#undef DEBUG
> 
> 
> Not needed, we do this in the makefile now.
> 
> 
>>+	if (debug >= 1)
>>+		pr_debug("%s: client=%p, dt=%p\n", __FUNCTION__, client, dt);
> 
> 
> Please use the dev_dbg(), dev_err() and friend functions instead of
> pr_debug().  It provides a sane user interface that all of the other
> drivers use.
> 
> thanks,
> 
> greg k-h

[-- Attachment #2: ds1337.patch --]
[-- Type: text/plain, Size: 11096 bytes --]

diff -Nru a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
--- a/drivers/i2c/chips/Kconfig	2005-02-27 20:42:22 +00:00
+++ b/drivers/i2c/chips/Kconfig	2005-02-27 20:42:22 +00:00
@@ -62,6 +62,17 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called asb100.
 
+config SENSORS_DS1337
+      	tristate "Dallas Semiconductor DS1337 Real Time Clock"
+	depends on I2C && EXPERIMENTAL
+	select I2C_SENSOR
+	help
+	  If you say yes here you get support for Dallas Semiconductor
+	  DS1337 real-time clock chips. 
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ds1337.
+
 config SENSORS_DS1621
       	tristate "Dallas Semiconductor DS1621 and DS1625"
 	depends on I2C && EXPERIMENTAL
diff -Nru a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
--- a/drivers/i2c/chips/Makefile	2005-02-27 20:42:22 +00:00
+++ b/drivers/i2c/chips/Makefile	2005-02-27 20:42:22 +00:00
@@ -11,6 +11,7 @@
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
+obj-$(CONFIG_SENSORS_DS1337)	+= ds1337.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o
 obj-$(CONFIG_SENSORS_FSCHER)	+= fscher.o
diff -Nru a/drivers/i2c/chips/ds1337.c b/drivers/i2c/chips/ds1337.c
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/drivers/i2c/chips/ds1337.c	2005-02-27 20:42:22 +00:00
@@ -0,0 +1,384 @@
+/*
+ *  linux/drivers/i2c/chips/ds1337.c
+ *
+ *  Copyright (C) 2005 James Chapman <jchapman@katalix.com>
+ *
+ *	based on linux/drivers/acron/char/pcf8583.c
+ *  Copyright (C) 2000 Russell King
+ *
+ * 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.
+ *
+ * Driver for Dallas Semiconductor DS1337 real time clock chip
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sensor.h>
+#include <linux/string.h>
+#include <linux/rtc.h>		/* get the user-level API */
+#include <linux/bcd.h>
+#include <linux/list.h>
+
+#define DS1337_NUM_REGS		16
+
+/* FIXME - how do we export these interface constants? */
+#define DS1337_GET_DATE		0
+#define DS1337_SET_DATE		1
+
+/*
+ * Functions declaration
+ */
+static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+SENSORS_INSMOD_1(ds1337);
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter);
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
+static void ds1337_init_client(struct i2c_client *client);
+static int ds1337_detach_client(struct i2c_client *client);
+static int ds1337_command(struct i2c_client *client, unsigned int cmd, void *arg);
+
+/*
+ * Driver data (common to all clients)
+ */
+static struct i2c_driver ds1337_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "ds1337",
+	.id		= I2C_DRIVERID_DS1337,
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= ds1337_attach_adapter,
+	.detach_client	= ds1337_detach_client,
+	.command	= ds1337_command,
+};
+
+/*
+ * Client data (each client gets its own)
+ */
+struct ds1337_data {
+	struct i2c_client client;
+	struct list_head list;
+	int id;
+};
+
+/*
+ * Internal variables
+ */
+static int ds1337_id = 0;
+static LIST_HEAD(ds1337_clients);
+
+static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
+{
+	s32 tmp = i2c_smbus_read_byte_data(client, reg);
+
+	if (tmp < 0)
+		return -EIO;
+
+	*value = (u8) (tmp & 0xff);
+
+	return 0;
+}
+
+/*
+ * Chip access functions
+ */
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+	int result;
+	u8 buf[7];
+	u8 val;
+	struct i2c_msg msg[2];
+	u8 offs = 0;
+
+	if (!dt || !client) {
+		dev_dbg(&client->adapter->dev, "%s: EINVAL: client=%p, dt=%p\n", 
+			__FUNCTION__, client, dt);
+
+		return -EINVAL;
+	}
+
+	memset(buf, 0, sizeof(buf));
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = 1;
+	msg[0].buf = &offs;
+
+	msg[1].addr = client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].len = sizeof(buf);
+	msg[1].buf = &buf[0];
+
+	result = client->adapter->algo->master_xfer(client->adapter,
+						    &msg[0],
+						    2);
+
+	dev_dbg(&client->adapter->dev, 
+		"%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
+		__FUNCTION__, result, buf[0], buf[1], buf[2], buf[3],
+		buf[4], buf[5], buf[6]);
+
+	if (result >= 0) {
+		dt->tm_sec = BCD_TO_BIN(buf[0]);
+		dt->tm_min = BCD_TO_BIN(buf[1]);
+		val = buf[2] & 0x3f;
+		dt->tm_hour = BCD_TO_BIN(val);
+		dt->tm_wday = BCD_TO_BIN(buf[3]) - 1;
+		dt->tm_mday = BCD_TO_BIN(buf[4]);
+		val = buf[5] & 0x7f;
+		dt->tm_mon = BCD_TO_BIN(val);
+		dt->tm_year = 1900 + BCD_TO_BIN(buf[6]);
+		if (buf[5] & 0x80)
+			dt->tm_year += 100;
+
+		dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, "
+			"hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
+			__FUNCTION__, dt->tm_sec, dt->tm_min,
+			dt->tm_hour, dt->tm_mday,
+			dt->tm_mon, dt->tm_year, dt->tm_wday);
+	} else {
+		dev_err(&client->adapter->dev, "ds1337[%d]: error reading "
+			"data! %d\n", client->id, result);
+		result = -EIO;
+	}
+
+	return result;
+}
+
+static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+	int result;
+	u8 buf[8];
+	u8 val;
+	struct i2c_msg msg[1];
+
+	if (!dt || !client) {
+		dev_dbg(&client->adapter->dev, "%s: EINVAL: client=%p, dt=%p\n", 
+			__FUNCTION__, client, dt);
+	
+		return -EINVAL;
+	}
+
+	dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, hours=%d, "
+		"mday=%d, mon=%d, year=%d, wday=%d\n", __FUNCTION__,
+		dt->tm_sec, dt->tm_min, dt->tm_hour,
+		dt->tm_mday, dt->tm_mon, dt->tm_year, dt->tm_wday);
+
+	buf[0] = 0;		/* reg offset */
+	buf[1] = BIN_TO_BCD(dt->tm_sec);
+	buf[2] = BIN_TO_BCD(dt->tm_min);
+	buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
+	buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
+	buf[5] = BIN_TO_BCD(dt->tm_mday);
+	buf[6] = BIN_TO_BCD(dt->tm_mon);
+	if (dt->tm_year >= 2000) {
+		val = dt->tm_year - 2000;
+		buf[6] |= (1 << 7);
+	} else {
+		val = dt->tm_year - 1900;
+	}
+	buf[7] = BIN_TO_BCD(val);
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = sizeof(buf);
+	msg[0].buf = &buf[0];
+
+	result = client->adapter->algo->master_xfer(client->adapter,
+						    &msg[0], 1);
+	if (result < 0) {
+		dev_err(&client->adapter->dev, "ds1337[%d]: error "
+			"writing data! %d\n", client->id, result);
+		result = -EIO;
+	} else {
+		result = 0;
+	}
+
+	return result;
+}
+
+static int ds1337_command(struct i2c_client *client, unsigned int cmd,
+			  void *arg)
+{
+	dev_dbg(&client->adapter->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+
+	switch (cmd) {
+	case DS1337_GET_DATE:
+		return ds1337_get_datetime(client, arg);
+
+	case DS1337_SET_DATE:
+		return ds1337_set_datetime(client, arg);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Public API for access to specific device. Useful for low-level
+ * RTC access from kernel code.
+ */
+int ds1337_do_command(int id, int cmd, void *arg)
+{
+	struct list_head *walk;
+	struct list_head *tmp;
+	struct ds1337_data *data;
+
+	list_for_each_safe(walk, tmp, &ds1337_clients) {
+		data = list_entry(walk, struct ds1337_data, list);
+		if (data->id == id) {
+			return ds1337_command(&data->client, cmd, arg);
+		}
+	}
+
+	return -ENODEV;
+}
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter)
+{
+	if (!(adapter->class & I2C_CLASS_HWMON))
+		return 0;
+	return i2c_detect(adapter, &addr_data, ds1337_detect);
+}
+
+/*
+ * The following function does more than just detection. If detection
+ * succeeds, it also registers the new chip.
+ */
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct ds1337_data *data;
+	int err = 0;
+	const char *name = "";
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		goto exit;
+
+	if (!(data = kmalloc(sizeof(struct ds1337_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct ds1337_data));
+	INIT_LIST_HEAD(&data->list);
+
+	err = -ENODEV;
+
+	/* The common I2C client data is placed right before the
+	   DS1337-specific data. */
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &ds1337_driver;
+	new_client->flags = 0;
+
+	/*
+	 * Now we do the remaining detection. A negative kind means that
+	 * the driver was loaded with no force parameter (default), so we
+	 * must both detect and identify the chip. A zero kind means that
+	 * the driver was loaded with the force parameter, the detection
+	 * step shall be skipped. A positive kind means that the driver
+	 * was loaded with the force parameter and a given kind of chip is
+	 * requested, so both the detection and the identification steps
+	 * are skipped.
+	 */
+
+	/* Default to an DS1337 if forced */
+	if (kind == 0)
+		kind = ds1337;
+
+	if (kind < 0) {		/* detection and identification */
+
+		u8 buf[DS1337_NUM_REGS];
+		int reg;
+
+		/* Check that all DS1337 registers are present */
+		for (reg = 0; reg < DS1337_NUM_REGS; reg++)
+			if (ds1337_read(new_client, reg, &buf[reg]) < 0)
+				goto exit_free;
+
+		/* Check that control register bits 5-6 are zero */
+		if (buf[14] & ((1 << 5) | (1 << 6)))
+			goto exit_free;
+
+		/* Check that status register bits 2-6 are zero */
+		if (buf[15] & ((1 << 2) | (1 << 3) | (1 << 4) |
+			       (1 << 5) | (1 << 6)))
+			goto exit_free;
+
+		kind = ds1337;
+	}
+
+	if (kind == ds1337)
+		name = "ds1337";
+
+	/* We can fill in the remaining client fields */
+	strlcpy(new_client->name, name, I2C_NAME_SIZE);
+	new_client->id = ds1337_id++;
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_free;
+
+	/* Initialize the DS1337 chip */
+	ds1337_init_client(new_client);
+
+	/* Add client to local list */
+	data->id = new_client->id;
+	list_add(&data->list, &ds1337_clients);
+
+	return 0;
+
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static void ds1337_init_client(struct i2c_client *client)
+{
+	u8 val;
+
+	/* Ensure that device is set in 24-hour mode */
+	val = i2c_smbus_read_byte_data(client, 2);
+	i2c_smbus_write_byte_data(client, 2, val | (1 << 6));
+}
+
+static int ds1337_detach_client(struct i2c_client *client)
+{
+	int err;
+	struct ds1337_data *data = i2c_get_clientdata(client);
+
+	if ((err = i2c_detach_client(client))) {
+		dev_err(&client->dev, "Client deregistration failed, "
+			"client not detached.\n");
+		return err;
+	}
+
+	list_del(&data->list);
+	kfree(data);
+	return 0;
+}
+
+static int __init ds1337_init(void)
+{
+	return i2c_add_driver(&ds1337_driver);
+}
+
+static void __exit ds1337_exit(void)
+{
+	i2c_del_driver(&ds1337_driver);
+}
+
+MODULE_AUTHOR("James Chapman <jchapman@katalix.com>");
+MODULE_DESCRIPTION("DS1337 RTC driver");
+MODULE_LICENSE("GPL");
+
+module_init(ds1337_init);
+module_exit(ds1337_exit);

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[Jean Delvare]

Hi James,

> diff -Nru a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
> --- a/drivers/i2c/chips/Kconfig	2005-02-27 20:42:22 +00:00
> +++ b/drivers/i2c/chips/Kconfig	2005-02-27 20:42:22 +00:00
> @@ -62,6 +62,17 @@
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called asb100.
> 
> +config SENSORS_DS1337
> +      	tristate "Dallas Semiconductor DS1337 Real Time Clock"
> +	depends on I2C && EXPERIMENTAL
> +	select I2C_SENSOR
> +	help
> +	  If you say yes here you get support for Dallas Semiconductor
> +	  DS1337 real-time clock chips.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ds1337.
> +
>  config SENSORS_DS1621
>        	tristate "Dallas Semiconductor DS1621 and DS1625"
>  	depends on I2C && EXPERIMENTAL

I don't think it belongs there. The DS1337 is not a hardware monitoring
chip, please move it down to the "Other I2C Chip support" menu. I also
think I see an indentation issue on the "tristate" line, seemingly
copied from the SENSORS_DS1621 section which would need to be fixed as
well.

> +static struct i2c_driver ds1337_driver = {
> +	.owner		= THIS_MODULE,
> +	.name		= "ds1337",
> +	.id		= I2C_DRIVERID_DS1337,
> +	.flags		= I2C_DF_NOTIFY,
> +	.attach_adapter	= ds1337_attach_adapter,
> +	.detach_client	= ds1337_detach_client,
> +	.command	= ds1337_command,
> +};

I2C_DRIVERID_DS1337 isn't defined anywhere that I can see, so your
driver will hardly compile. You don't seem to actually need a driver
id, so you can simply omit it.

> +static inline int ds1337_read(struct i2c_client *client, u8 reg, u8
> 	*value)
> +{
> +	s32 tmp = i2c_smbus_read_byte_data(client, reg);
> +
> +	if (tmp < 0)
> +		return -EIO;
> +
> +	*value = (u8) (tmp & 0xff);
> +
> +	return 0;
> +}

The bitmasking is a no-op, and so is probably the cast as I'd expect the
compiler to do the right thing on its own.

> +static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time > 	*dt)
> +{
> (...)
> +	if (!dt || !client) {

How could client be NULL? Same question for ds1337_set_datetime.

> +	memset(buf, 0, sizeof(buf));

Why is this memset needed at all?

> +	result = client->adapter->algo->master_xfer(client->adapter,
> +						    &msg[0],
> +						    2);

You did not check that the adapter was capable of raw I2C transfers.

> +static int ds1337_attach_adapter(struct i2c_adapter *adapter)
> +{
> +	if (!(adapter->class & I2C_CLASS_HWMON))
> +		return 0;

You do not want to check that. The DS1337 is not a hardware monitoring
device!

> +static int ds1337_detect(struct i2c_adapter *adapter, int address, int
>	kind)
+{
+	struct i2c_client *new_client;
+	struct ds1337_data *data;
+	int err = 0;
+	const char *name = "";
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		goto exit;

You must verify the ability to do raw I2C transfers here, as said above.

+	err = -ENODEV;

Not correct. The detection function should NOT return a non-zero value
unless it meets a critical error (such as memory shortage). Just finding
a different device from what you were looking for isn't an error.

> +		/* Check that control register bits 5-6 are zero */
> +		if (buf[14] & ((1 << 5) | (1 << 6)))
> +			goto exit_free;
> +
> +		/* Check that status register bits 2-6 are zero */
> +		if (buf[15] & ((1 << 2) | (1 << 3) | (1 << 4) |
> +			       (1 << 5) | (1 << 6)))
> +			goto exit_free;

These would be much more readable as "& 0x60" and "& 0x7c" if you
want my opinion (but maybe it's just me). Also, I usually write "bits
6-2", not "2-6" (but again that might be just me).

> +	new_client->id = ds1337_id++;

Client ids are not used by the core and were dropped recently. Don't use
that struct member.

> +static void ds1337_init_client(struct i2c_client *client)
> +{
> +	u8 val;
> +
> +	/* Ensure that device is set in 24-hour mode */
> +	val = i2c_smbus_read_byte_data(client, 2);
> +	i2c_smbus_write_byte_data(client, 2, val | (1 << 6));
> +}

You can probably spare a write if the device was already in 24-hour mode.
It might also be nice to give register 2 a name (DS1337_REG_CONFIG?) and
use that.

Also note that you are not handling errors properly here. You could get
-1 returned from the read, forcing all bits of the register to 1 on
subsequent write.

--
Jean Delvare

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[Greg KH]

On Tue, Mar 01, 2005 at 07:21:56PM +0000, James Chapman wrote:
> Revised ds1337 chip driver patch.
> 
> Signed-off-by: James Chapman <jchapman@katalix.com>
> 
> - change all driver log messages to use dev_dbg() or dev_err()
> - remove debug module parameter

Hm, doesn't seem to apply at all:

drivers/i2c/chips/Kconfig 1.78: 418 lines
patching file drivers/i2c/chips/Kconfig
drivers/i2c/chips/Makefile 1.61: 46 lines
patching file drivers/i2c/chips/Makefile
patching file drivers/i2c/chips/ds1337.c
patch: **** malformed patch at line 420: 


Care to respin it again?

thanks,

greg k-h

[PATCH 2.6] Trivial indentation fix in i2c/chips/Kconfig

[Jean Delvare]

Hi Greg,

Quoting myself:

> (...) I also think I see an indentation issue on the "tristate" line,
> seemingly copied from the SENSORS_DS1621 section which would need to
> be fixed as well.

Here is the trivial patch fixing that, if you want to apply it.

Thanks,

Signed-off-by: Jean Delvare <khali@linux-fr.org>

--- linux-2.6.11/drivers/i2c/chips/Kconfig.orig	Wed Mar  2 15:12:34 2005
+++ linux-2.6.11/drivers/i2c/chips/Kconfig	Wed Mar  2 15:15:12 2005
@@ -63,7 +63,7 @@
 	  will be called asb100.
 
 config SENSORS_DS1621
-      	tristate "Dallas Semiconductor DS1621 and DS1625"
+	tristate "Dallas Semiconductor DS1621 and DS1625"
 	depends on I2C && EXPERIMENTAL
 	select I2C_SENSOR
 	help


-- 
Jean Delvare

Re: [PATCH 2.6] Trivial indentation fix in i2c/chips/Kconfig

[Greg KH]

On Wed, Mar 02, 2005 at 07:45:22PM +0100, Jean Delvare wrote:
> Hi Greg,
> 
> Quoting myself:
> 
> > (...) I also think I see an indentation issue on the "tristate" line,
> > seemingly copied from the SENSORS_DS1621 section which would need to
> > be fixed as well.
> 
> Here is the trivial patch fixing that, if you want to apply it.
> 
> Thanks,
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>

Applied, thanks.

greg k-h

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[Andrew Morton]

James Chapman <jchapman@katalix.com> wrote:
>
> Add DS1337 RTC chip driver.
> 

drivers/i2c/chips/ds1337.c:60: `I2C_DRIVERID_DS1337' undeclared here (not in a function)


Also, there are changes in Greg's i2c tree which break your new driver:

drivers/i2c/chips/ds1337.c:60: initializer element is not constant
drivers/i2c/chips/ds1337.c:60: (near initialization for `ds1337_driver.id')
drivers/i2c/chips/ds1337.c: In function `ds1337_get_datetime':
drivers/i2c/chips/ds1337.c:155: structure has no member named `id'
drivers/i2c/chips/ds1337.c: In function `ds1337_set_datetime':
drivers/i2c/chips/ds1337.c:206: structure has no member named `id'
drivers/i2c/chips/ds1337.c: In function `ds1337_detect':
drivers/i2c/chips/ds1337.c:333: structure has no member named `id'
drivers/i2c/chips/ds1337.c:343: structure has no member named `id'

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[James Chapman]

[-- Attachment #1: Type: text/plain, Size: 70 bytes --]

A revised ds1337 patch addressing all of Jean's comments is attached.

[-- Attachment #2: ds1337.patch --]
[-- Type: text/plain, Size: 11288 bytes --]

i2c: add ds1337 RTC chip support

Signed-off-by: James Chapman <jchapman@katalix.com>

Index: linux-2.6.i2c/drivers/i2c/chips/ds1337.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.i2c/drivers/i2c/chips/ds1337.c	2005-03-03 16:33:37.000000000 +0000
@@ -0,0 +1,380 @@
+/*
+ *  linux/drivers/i2c/chips/ds1337.c
+ *
+ *  Copyright (C) 2005 James Chapman <jchapman@katalix.com>
+ *
+ *	based on linux/drivers/acron/char/pcf8583.c
+ *  Copyright (C) 2000 Russell King
+ *
+ * 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.
+ *
+ * Driver for Dallas Semiconductor DS1337 real time clock chip
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sensor.h>
+#include <linux/string.h>
+#include <linux/rtc.h>		/* get the user-level API */
+#include <linux/bcd.h>
+#include <linux/list.h>
+
+#define DS1337_NUM_REGS		16
+#define DS1337_REG_CONFIG	2
+
+/* FIXME - how do we export these interface constants? */
+#define DS1337_GET_DATE		0
+#define DS1337_SET_DATE		1
+
+/*
+ * Functions declaration
+ */
+static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+SENSORS_INSMOD_1(ds1337);
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter);
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
+static void ds1337_init_client(struct i2c_client *client);
+static int ds1337_detach_client(struct i2c_client *client);
+static int ds1337_command(struct i2c_client *client, unsigned int cmd,
+			  void *arg);
+
+/*
+ * Driver data (common to all clients)
+ */
+static struct i2c_driver ds1337_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "ds1337",
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= ds1337_attach_adapter,
+	.detach_client	= ds1337_detach_client,
+	.command	= ds1337_command,
+};
+
+/*
+ * Client data (each client gets its own)
+ */
+struct ds1337_data {
+	struct i2c_client client;
+	struct list_head list;
+	int id;
+};
+
+/*
+ * Internal variables
+ */
+static int ds1337_id;
+static LIST_HEAD(ds1337_clients);
+
+static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
+{
+	s32 tmp = i2c_smbus_read_byte_data(client, reg);
+
+	if (tmp < 0)
+		return -EIO;
+
+	*value = tmp;
+
+	return 0;
+}
+
+/*
+ * Chip access functions
+ */
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+	struct ds1337_data *data = i2c_get_clientdata(client);
+	int result;
+	u8 buf[7];
+	u8 val;
+	struct i2c_msg msg[2];
+	u8 offs = 0;
+
+	if (!dt) {
+		dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
+			__FUNCTION__);
+
+		return -EINVAL;
+	}
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = 1;
+	msg[0].buf = &offs;
+
+	msg[1].addr = client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].len = sizeof(buf);
+	msg[1].buf = &buf[0];
+
+	result = client->adapter->algo->master_xfer(client->adapter,
+						    &msg[0], 2);
+
+	dev_dbg(&client->adapter->dev,
+		"%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
+		__FUNCTION__, result, buf[0], buf[1], buf[2], buf[3],
+		buf[4], buf[5], buf[6]);
+
+	if (result >= 0) {
+		dt->tm_sec = BCD_TO_BIN(buf[0]);
+		dt->tm_min = BCD_TO_BIN(buf[1]);
+		val = buf[2] & 0x3f;
+		dt->tm_hour = BCD_TO_BIN(val);
+		dt->tm_wday = BCD_TO_BIN(buf[3]) - 1;
+		dt->tm_mday = BCD_TO_BIN(buf[4]);
+		val = buf[5] & 0x7f;
+		dt->tm_mon = BCD_TO_BIN(val);
+		dt->tm_year = 1900 + BCD_TO_BIN(buf[6]);
+		if (buf[5] & 0x80)
+			dt->tm_year += 100;
+
+		dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, "
+			"hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
+			__FUNCTION__, dt->tm_sec, dt->tm_min,
+			dt->tm_hour, dt->tm_mday,
+			dt->tm_mon, dt->tm_year, dt->tm_wday);
+	} else {
+		dev_err(&client->adapter->dev, "ds1337[%d]: error reading "
+			"data! %d\n", data->id, result);
+		result = -EIO;
+	}
+
+	return result;
+}
+
+static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+	struct ds1337_data *data = i2c_get_clientdata(client);
+	int result;
+	u8 buf[8];
+	u8 val;
+	struct i2c_msg msg[1];
+
+	if (!dt) {
+		dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
+			__FUNCTION__);
+
+		return -EINVAL;
+	}
+
+	dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, hours=%d, "
+		"mday=%d, mon=%d, year=%d, wday=%d\n", __FUNCTION__,
+		dt->tm_sec, dt->tm_min, dt->tm_hour,
+		dt->tm_mday, dt->tm_mon, dt->tm_year, dt->tm_wday);
+
+	buf[0] = 0;		/* reg offset */
+	buf[1] = BIN_TO_BCD(dt->tm_sec);
+	buf[2] = BIN_TO_BCD(dt->tm_min);
+	buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
+	buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
+	buf[5] = BIN_TO_BCD(dt->tm_mday);
+	buf[6] = BIN_TO_BCD(dt->tm_mon);
+	if (dt->tm_year >= 2000) {
+		val = dt->tm_year - 2000;
+		buf[6] |= (1 << 7);
+	} else {
+		val = dt->tm_year - 1900;
+	}
+	buf[7] = BIN_TO_BCD(val);
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = sizeof(buf);
+	msg[0].buf = &buf[0];
+
+	result = client->adapter->algo->master_xfer(client->adapter,
+						    &msg[0], 1);
+	if (result < 0) {
+		dev_err(&client->adapter->dev, "ds1337[%d]: error "
+			"writing data! %d\n", data->id, result);
+		result = -EIO;
+	} else {
+		result = 0;
+	}
+
+	return result;
+}
+
+static int ds1337_command(struct i2c_client *client, unsigned int cmd,
+			  void *arg)
+{
+	dev_dbg(&client->adapter->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+
+	switch (cmd) {
+	case DS1337_GET_DATE:
+		return ds1337_get_datetime(client, arg);
+
+	case DS1337_SET_DATE:
+		return ds1337_set_datetime(client, arg);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Public API for access to specific device. Useful for low-level
+ * RTC access from kernel code.
+ */
+int ds1337_do_command(int id, int cmd, void *arg)
+{
+	struct list_head *walk;
+	struct list_head *tmp;
+	struct ds1337_data *data;
+
+	list_for_each_safe(walk, tmp, &ds1337_clients) {
+		data = list_entry(walk, struct ds1337_data, list);
+		if (data->id == id)
+			return ds1337_command(&data->client, cmd, arg);
+	}
+
+	return -ENODEV;
+}
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_detect(adapter, &addr_data, ds1337_detect);
+}
+
+/*
+ * The following function does more than just detection. If detection
+ * succeeds, it also registers the new chip.
+ */
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct ds1337_data *data;
+	int err = 0;
+	const char *name = "";
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
+				     I2C_FUNC_SMBUS_I2C_BLOCK))
+		goto exit;
+
+	if (!(data = kmalloc(sizeof(struct ds1337_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct ds1337_data));
+	INIT_LIST_HEAD(&data->list);
+
+	/* The common I2C client data is placed right before the
+	 * DS1337-specific data. 
+	 */
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &ds1337_driver;
+	new_client->flags = 0;
+
+	/*
+	 * Now we do the remaining detection. A negative kind means that
+	 * the driver was loaded with no force parameter (default), so we
+	 * must both detect and identify the chip. A zero kind means that
+	 * the driver was loaded with the force parameter, the detection
+	 * step shall be skipped. A positive kind means that the driver
+	 * was loaded with the force parameter and a given kind of chip is
+	 * requested, so both the detection and the identification steps
+	 * are skipped.
+	 */
+
+	/* Default to an DS1337 if forced */
+	if (kind == 0)
+		kind = ds1337;
+
+	if (kind < 0) {		/* detection and identification */
+
+		u8 buf[DS1337_NUM_REGS];
+		int reg;
+
+		/* Check that all DS1337 registers are present */
+		for (reg = 0; reg < DS1337_NUM_REGS; reg++)
+			if (ds1337_read(new_client, reg, &buf[reg]) < 0)
+				goto exit_free;
+
+		/* Check that control register bits 6-5 are zero */
+		if (buf[14] & 0x60)
+			goto exit_free;
+
+		/* Check that status register bits 6-2 are zero */
+		if (buf[15] & 0x7c)
+			goto exit_free;
+
+		kind = ds1337;
+	}
+
+	if (kind == ds1337)
+		name = "ds1337";
+
+	/* We can fill in the remaining client fields */
+	strlcpy(new_client->name, name, I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_free;
+
+	/* Initialize the DS1337 chip */
+	ds1337_init_client(new_client);
+
+	/* Add client to local list */
+	data->id = ds1337_id++;
+	list_add(&data->list, &ds1337_clients);
+
+	return 0;
+
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static void ds1337_init_client(struct i2c_client *client)
+{
+	s32 val;
+
+	/* Ensure that device is set in 24-hour mode */
+	val = i2c_smbus_read_byte_data(client, DS1337_REG_CONFIG);
+	if ((val >= 0) && (val & (1 << 6)) == 0)
+		i2c_smbus_write_byte_data(client, DS1337_REG_CONFIG,
+					  val | (1 << 6));
+}
+
+static int ds1337_detach_client(struct i2c_client *client)
+{
+	int err;
+	struct ds1337_data *data = i2c_get_clientdata(client);
+
+	if ((err = i2c_detach_client(client))) {
+		dev_err(&client->dev, "Client deregistration failed, "
+			"client not detached.\n");
+		return err;
+	}
+
+	list_del(&data->list);
+	kfree(data);
+	return 0;
+}
+
+static int __init ds1337_init(void)
+{
+	return i2c_add_driver(&ds1337_driver);
+}
+
+static void __exit ds1337_exit(void)
+{
+	i2c_del_driver(&ds1337_driver);
+}
+
+MODULE_AUTHOR("James Chapman <jchapman@katalix.com>");
+MODULE_DESCRIPTION("DS1337 RTC driver");
+MODULE_LICENSE("GPL");
+
+module_init(ds1337_init);
+module_exit(ds1337_exit);
Index: linux-2.6.i2c/drivers/i2c/chips/Kconfig
===================================================================
--- linux-2.6.i2c.orig/drivers/i2c/chips/Kconfig	2005-03-03 15:34:27.000000000 +0000
+++ linux-2.6.i2c/drivers/i2c/chips/Kconfig	2005-03-03 15:51:59.000000000 +0000
@@ -320,6 +320,17 @@
 menu "Other I2C Chip support"
 	depends on I2C
 
+config SENSORS_DS1337
+	tristate "Dallas Semiconductor DS1337 Real Time Clock"
+	depends on I2C && EXPERIMENTAL
+	select I2C_SENSOR
+	help
+	  If you say yes here you get support for Dallas Semiconductor
+	  DS1337 real-time clock chips. 
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ds1337.
+
 config SENSORS_EEPROM
 	tristate "EEPROM reader"
 	depends on I2C && EXPERIMENTAL
Index: linux-2.6.i2c/drivers/i2c/chips/Makefile
===================================================================
--- linux-2.6.i2c.orig/drivers/i2c/chips/Makefile	2005-03-03 15:02:40.000000000 +0000
+++ linux-2.6.i2c/drivers/i2c/chips/Makefile	2005-03-03 15:53:11.000000000 +0000
@@ -11,6 +11,7 @@
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
+obj-$(CONFIG_SENSORS_DS1337)	+= ds1337.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o
 obj-$(CONFIG_SENSORS_FSCHER)	+= fscher.o

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[Jean Delvare]

Hi James,

> A revised ds1337 patch addressing all of Jean's comments is attached.

Fine with me except for:

> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
> +				     I2C_FUNC_SMBUS_I2C_BLOCK))

I don't this it is correct. You are using master_xfer, not
i2c_smbus_{read,write}_i2c_block_data. Adapter which declare themselves
I2C_FUNC_SMBUS_I2C_BLOCK-capable may not implement master_xfer. You
really need to check for I2C_FUNC_I2C.

Now I agree that the transfers you do ARE i2c block transfers, and I
find it highly questionable that our implementation of
i2c_smbus_read_i2c_block_data will always read 32 bytes of data from the
chip. It would be much more convenient to allow I2C block reads of
arbitrary length, (just like we do with I2C block writes) so that
clients can use this function instead of master_xfer.

It should be a quite simple fix if I correctly remember the i2c-core
code, with the only drawback that it alters the API. That being said,
the only kernel user (in kernel) of this function that I could find is
the eeprom driver (this can be easily explained by the fact that this
function, as it is now, is essentially useless), so I wouldn't mind the
risk. The net benefit would be that i2c chip drivers could start using
this function instead of master_xfer, so they would possibly work with
more than just the fully I2C-capable adapters (not that many of them,
see list right below).

i2c-dev might be a problem if we go that way, because we will silently
change the way I2C block reads requested from userspace are handled. Not
sure it is a big issue though, because as underlined before, the
function as it is now is rather useless. I doubt that anything but
i2cdump uses it in userspace.

Then we would need to fix bus drivers that implement the call by
themselves (as opposed to emulation), but in fact only a few of them do
(i2c-amd8111 and i2c-nforce2) so that should be quickly done.
[Reading the two bus drivers code...]

And it turns out that both bus drivers ALREADY honor the length
requested by the caller, which is not consistent with the emulated
variant of the call. Something definitely must be done.

Any thoughts anyone?

Thanks,
-- 
Jean Delvare

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[James Chapman]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

Revised patch is attached.

Jean Delvare wrote:

> Hi James,
> 
> 
>>A revised ds1337 patch addressing all of Jean's comments is attached.
> 
> 
> Fine with me except for:
> 
> 
>>+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
>>+				     I2C_FUNC_SMBUS_I2C_BLOCK))
> 
> 
> I don't this it is correct. You are using master_xfer, not
> i2c_smbus_{read,write}_i2c_block_data. Adapter which declare themselves
> I2C_FUNC_SMBUS_I2C_BLOCK-capable may not implement master_xfer. You
> really need to check for I2C_FUNC_I2C.
> 

[-- Attachment #2: ds1337.patch --]
[-- Type: text/plain, Size: 11276 bytes --]

i2c: add ds1337 RTC chip support

Signed-off-by: James Chapman <jchapman@katalix.com>

Index: linux-2.6.i2c/drivers/i2c/chips/ds1337.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.i2c/drivers/i2c/chips/ds1337.c	2005-03-04 00:01:44.000000000 +0000
@@ -0,0 +1,380 @@
+/*
+ *  linux/drivers/i2c/chips/ds1337.c
+ *
+ *  Copyright (C) 2005 James Chapman <jchapman@katalix.com>
+ *
+ *	based on linux/drivers/acron/char/pcf8583.c
+ *  Copyright (C) 2000 Russell King
+ *
+ * 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.
+ *
+ * Driver for Dallas Semiconductor DS1337 real time clock chip
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sensor.h>
+#include <linux/string.h>
+#include <linux/rtc.h>		/* get the user-level API */
+#include <linux/bcd.h>
+#include <linux/list.h>
+
+#define DS1337_NUM_REGS		16
+#define DS1337_REG_CONFIG	2
+
+/* FIXME - how do we export these interface constants? */
+#define DS1337_GET_DATE		0
+#define DS1337_SET_DATE		1
+
+/*
+ * Functions declaration
+ */
+static unsigned short normal_i2c[] = { 0x68, I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+SENSORS_INSMOD_1(ds1337);
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter);
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind);
+static void ds1337_init_client(struct i2c_client *client);
+static int ds1337_detach_client(struct i2c_client *client);
+static int ds1337_command(struct i2c_client *client, unsigned int cmd,
+			  void *arg);
+
+/*
+ * Driver data (common to all clients)
+ */
+static struct i2c_driver ds1337_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "ds1337",
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= ds1337_attach_adapter,
+	.detach_client	= ds1337_detach_client,
+	.command	= ds1337_command,
+};
+
+/*
+ * Client data (each client gets its own)
+ */
+struct ds1337_data {
+	struct i2c_client client;
+	struct list_head list;
+	int id;
+};
+
+/*
+ * Internal variables
+ */
+static int ds1337_id;
+static LIST_HEAD(ds1337_clients);
+
+static inline int ds1337_read(struct i2c_client *client, u8 reg, u8 *value)
+{
+	s32 tmp = i2c_smbus_read_byte_data(client, reg);
+
+	if (tmp < 0)
+		return -EIO;
+
+	*value = tmp;
+
+	return 0;
+}
+
+/*
+ * Chip access functions
+ */
+static int ds1337_get_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+	struct ds1337_data *data = i2c_get_clientdata(client);
+	int result;
+	u8 buf[7];
+	u8 val;
+	struct i2c_msg msg[2];
+	u8 offs = 0;
+
+	if (!dt) {
+		dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
+			__FUNCTION__);
+
+		return -EINVAL;
+	}
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = 1;
+	msg[0].buf = &offs;
+
+	msg[1].addr = client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].len = sizeof(buf);
+	msg[1].buf = &buf[0];
+
+	result = client->adapter->algo->master_xfer(client->adapter,
+						    &msg[0], 2);
+
+	dev_dbg(&client->adapter->dev,
+		"%s: [%d] %02x %02x %02x %02x %02x %02x %02x\n",
+		__FUNCTION__, result, buf[0], buf[1], buf[2], buf[3],
+		buf[4], buf[5], buf[6]);
+
+	if (result >= 0) {
+		dt->tm_sec = BCD_TO_BIN(buf[0]);
+		dt->tm_min = BCD_TO_BIN(buf[1]);
+		val = buf[2] & 0x3f;
+		dt->tm_hour = BCD_TO_BIN(val);
+		dt->tm_wday = BCD_TO_BIN(buf[3]) - 1;
+		dt->tm_mday = BCD_TO_BIN(buf[4]);
+		val = buf[5] & 0x7f;
+		dt->tm_mon = BCD_TO_BIN(val);
+		dt->tm_year = 1900 + BCD_TO_BIN(buf[6]);
+		if (buf[5] & 0x80)
+			dt->tm_year += 100;
+
+		dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, "
+			"hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
+			__FUNCTION__, dt->tm_sec, dt->tm_min,
+			dt->tm_hour, dt->tm_mday,
+			dt->tm_mon, dt->tm_year, dt->tm_wday);
+	} else {
+		dev_err(&client->adapter->dev, "ds1337[%d]: error reading "
+			"data! %d\n", data->id, result);
+		result = -EIO;
+	}
+
+	return result;
+}
+
+static int ds1337_set_datetime(struct i2c_client *client, struct rtc_time *dt)
+{
+	struct ds1337_data *data = i2c_get_clientdata(client);
+	int result;
+	u8 buf[8];
+	u8 val;
+	struct i2c_msg msg[1];
+
+	if (!dt) {
+		dev_dbg(&client->adapter->dev, "%s: EINVAL: dt=NULL\n",
+			__FUNCTION__);
+
+		return -EINVAL;
+	}
+
+	dev_dbg(&client->adapter->dev, "%s: secs=%d, mins=%d, hours=%d, "
+		"mday=%d, mon=%d, year=%d, wday=%d\n", __FUNCTION__,
+		dt->tm_sec, dt->tm_min, dt->tm_hour,
+		dt->tm_mday, dt->tm_mon, dt->tm_year, dt->tm_wday);
+
+	buf[0] = 0;		/* reg offset */
+	buf[1] = BIN_TO_BCD(dt->tm_sec);
+	buf[2] = BIN_TO_BCD(dt->tm_min);
+	buf[3] = BIN_TO_BCD(dt->tm_hour) | (1 << 6);
+	buf[4] = BIN_TO_BCD(dt->tm_wday) + 1;
+	buf[5] = BIN_TO_BCD(dt->tm_mday);
+	buf[6] = BIN_TO_BCD(dt->tm_mon);
+	if (dt->tm_year >= 2000) {
+		val = dt->tm_year - 2000;
+		buf[6] |= (1 << 7);
+	} else {
+		val = dt->tm_year - 1900;
+	}
+	buf[7] = BIN_TO_BCD(val);
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = sizeof(buf);
+	msg[0].buf = &buf[0];
+
+	result = client->adapter->algo->master_xfer(client->adapter,
+						    &msg[0], 1);
+	if (result < 0) {
+		dev_err(&client->adapter->dev, "ds1337[%d]: error "
+			"writing data! %d\n", data->id, result);
+		result = -EIO;
+	} else {
+		result = 0;
+	}
+
+	return result;
+}
+
+static int ds1337_command(struct i2c_client *client, unsigned int cmd,
+			  void *arg)
+{
+	dev_dbg(&client->adapter->dev, "%s: cmd=%d\n", __FUNCTION__, cmd);
+
+	switch (cmd) {
+	case DS1337_GET_DATE:
+		return ds1337_get_datetime(client, arg);
+
+	case DS1337_SET_DATE:
+		return ds1337_set_datetime(client, arg);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Public API for access to specific device. Useful for low-level
+ * RTC access from kernel code.
+ */
+int ds1337_do_command(int id, int cmd, void *arg)
+{
+	struct list_head *walk;
+	struct list_head *tmp;
+	struct ds1337_data *data;
+
+	list_for_each_safe(walk, tmp, &ds1337_clients) {
+		data = list_entry(walk, struct ds1337_data, list);
+		if (data->id == id)
+			return ds1337_command(&data->client, cmd, arg);
+	}
+
+	return -ENODEV;
+}
+
+static int ds1337_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_detect(adapter, &addr_data, ds1337_detect);
+}
+
+/*
+ * The following function does more than just detection. If detection
+ * succeeds, it also registers the new chip.
+ */
+static int ds1337_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct ds1337_data *data;
+	int err = 0;
+	const char *name = "";
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
+				     I2C_FUNC_I2C))
+		goto exit;
+
+	if (!(data = kmalloc(sizeof(struct ds1337_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct ds1337_data));
+	INIT_LIST_HEAD(&data->list);
+
+	/* The common I2C client data is placed right before the
+	 * DS1337-specific data. 
+	 */
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &ds1337_driver;
+	new_client->flags = 0;
+
+	/*
+	 * Now we do the remaining detection. A negative kind means that
+	 * the driver was loaded with no force parameter (default), so we
+	 * must both detect and identify the chip. A zero kind means that
+	 * the driver was loaded with the force parameter, the detection
+	 * step shall be skipped. A positive kind means that the driver
+	 * was loaded with the force parameter and a given kind of chip is
+	 * requested, so both the detection and the identification steps
+	 * are skipped.
+	 */
+
+	/* Default to an DS1337 if forced */
+	if (kind == 0)
+		kind = ds1337;
+
+	if (kind < 0) {		/* detection and identification */
+
+		u8 buf[DS1337_NUM_REGS];
+		int reg;
+
+		/* Check that all DS1337 registers are present */
+		for (reg = 0; reg < DS1337_NUM_REGS; reg++)
+			if (ds1337_read(new_client, reg, &buf[reg]) < 0)
+				goto exit_free;
+
+		/* Check that control register bits 6-5 are zero */
+		if (buf[14] & 0x60)
+			goto exit_free;
+
+		/* Check that status register bits 6-2 are zero */
+		if (buf[15] & 0x7c)
+			goto exit_free;
+
+		kind = ds1337;
+	}
+
+	if (kind == ds1337)
+		name = "ds1337";
+
+	/* We can fill in the remaining client fields */
+	strlcpy(new_client->name, name, I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_free;
+
+	/* Initialize the DS1337 chip */
+	ds1337_init_client(new_client);
+
+	/* Add client to local list */
+	data->id = ds1337_id++;
+	list_add(&data->list, &ds1337_clients);
+
+	return 0;
+
+exit_free:
+	kfree(data);
+exit:
+	return err;
+}
+
+static void ds1337_init_client(struct i2c_client *client)
+{
+	s32 val;
+
+	/* Ensure that device is set in 24-hour mode */
+	val = i2c_smbus_read_byte_data(client, DS1337_REG_CONFIG);
+	if ((val >= 0) && (val & (1 << 6)) == 0)
+		i2c_smbus_write_byte_data(client, DS1337_REG_CONFIG,
+					  val | (1 << 6));
+}
+
+static int ds1337_detach_client(struct i2c_client *client)
+{
+	int err;
+	struct ds1337_data *data = i2c_get_clientdata(client);
+
+	if ((err = i2c_detach_client(client))) {
+		dev_err(&client->dev, "Client deregistration failed, "
+			"client not detached.\n");
+		return err;
+	}
+
+	list_del(&data->list);
+	kfree(data);
+	return 0;
+}
+
+static int __init ds1337_init(void)
+{
+	return i2c_add_driver(&ds1337_driver);
+}
+
+static void __exit ds1337_exit(void)
+{
+	i2c_del_driver(&ds1337_driver);
+}
+
+MODULE_AUTHOR("James Chapman <jchapman@katalix.com>");
+MODULE_DESCRIPTION("DS1337 RTC driver");
+MODULE_LICENSE("GPL");
+
+module_init(ds1337_init);
+module_exit(ds1337_exit);
Index: linux-2.6.i2c/drivers/i2c/chips/Kconfig
===================================================================
--- linux-2.6.i2c.orig/drivers/i2c/chips/Kconfig	2005-03-03 23:15:54.000000000 +0000
+++ linux-2.6.i2c/drivers/i2c/chips/Kconfig	2005-03-04 00:01:15.000000000 +0000
@@ -320,6 +320,17 @@
 menu "Other I2C Chip support"
 	depends on I2C
 
+config SENSORS_DS1337
+	tristate "Dallas Semiconductor DS1337 Real Time Clock"
+	depends on I2C && EXPERIMENTAL
+	select I2C_SENSOR
+	help
+	  If you say yes here you get support for Dallas Semiconductor
+	  DS1337 real-time clock chips. 
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ds1337.
+
 config SENSORS_EEPROM
 	tristate "EEPROM reader"
 	depends on I2C && EXPERIMENTAL
Index: linux-2.6.i2c/drivers/i2c/chips/Makefile
===================================================================
--- linux-2.6.i2c.orig/drivers/i2c/chips/Makefile	2005-03-03 23:15:54.000000000 +0000
+++ linux-2.6.i2c/drivers/i2c/chips/Makefile	2005-03-04 00:01:15.000000000 +0000
@@ -11,6 +11,7 @@
 obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
+obj-$(CONFIG_SENSORS_DS1337)	+= ds1337.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o
 obj-$(CONFIG_SENSORS_FSCHER)	+= fscher.o

Re: [PATCH: 2.6.11-rc5] i2c chips: ds1337 RTC driver

[Jean Delvare]

Hi James, all,

> Revised patch is attached.

Looks all OK to me.

Thanks,
--
Jean Delvare