[PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Randy Vinson]

Greetings,
  I've put together a small I2C client for the Maxim/Dallas DS1374 RTC
chip and tested it on a Freescale MPC8349ADS board that uses the chip.
The attached patch adds support for the chip itself and a follow-up patch
will add support to the Freescale board.

  Please CC: me on any replies as I am not subscribed to the sensors list.

		Thanks,
		Randy Vinson


Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

This change adds support for the Maxim/Dallas DS1374 RTC chip. This chip
is an I2C-based RTC that maintains a simple 32-bit binary seconds count
with battery backup support.

Signed-off-by: Randy Vinson <rvinson@mvista.com>

---
commit f95cc4d276cc332225bd823a2ae3be553c119990
tree bd11f3262e29c13b6d56383415c634371394e913
parent b56ae7a07ed0304bbc1dd61d3527dccdbcc467e9
author Randy Vinson <rvinson@linuxbox.(none)> Fri, 03 Jun 2005 13:50:59 -0700
committer Randy Vinson <rvinson@linuxbox.(none)> Fri, 03 Jun 2005 13:50:59 -0700

 drivers/i2c/chips/Kconfig  |   11 ++
 drivers/i2c/chips/Makefile |    1 
 drivers/i2c/chips/ds1374.c |  266 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c-id.h     |    1 
 4 files changed, 279 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
--- a/drivers/i2c/chips/Kconfig
+++ b/drivers/i2c/chips/Kconfig
@@ -376,6 +376,17 @@ config SENSORS_DS1337
 	  This driver can also be built as a module.  If so, the module
 	  will be called ds1337.
 
+config SENSORS_DS1374
+	tristate "Maxim/Dallas Semiconductor DS1374 Real Time Clock"
+	depends on I2C && EXPERIMENTAL
+	select I2C_SENSOR
+	help
+	  If you say yes here you get support for Dallas Semiconductor
+	  DS1374 real-time clock chips. 
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ds1374.
+
 config SENSORS_EEPROM
 	tristate "EEPROM reader"
 	depends on I2C && EXPERIMENTAL
diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
--- a/drivers/i2c/chips/Makefile
+++ b/drivers/i2c/chips/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_DS1337)	+= ds1337.o
+obj-$(CONFIG_SENSORS_DS1374)	+= ds1374.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o
 obj-$(CONFIG_SENSORS_FSCHER)	+= fscher.o
diff --git a/drivers/i2c/chips/ds1374.c b/drivers/i2c/chips/ds1374.c
new file mode 100644
--- /dev/null
+++ b/drivers/i2c/chips/ds1374.c
@@ -0,0 +1,266 @@
+/*
+ * drivers/i2c/chips/ds1374.c
+ *
+ * I2C client/driver for the Maxim/Dallas DS1374 Real-Time Clock
+ *
+ * Author: Randy Vinson <rvinson@mvista.com>
+ *
+ * Based on the m41t00.c by Mark Greer <mgreer@mvista.com>
+ *
+ * 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.
+ */
+/*
+ * This i2c client/driver wedges between the drivers/char/genrtc.c RTC
+ * interface and the SMBus interface of the i2c subsystem.
+ * It would be more efficient to use i2c msgs/i2c_transfer directly but, as
+ * recommened in .../Documentation/i2c/writing-clients section
+ * "Sending and receiving", using SMBus level communication is preferred.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/rtc.h>
+#include <linux/bcd.h>
+
+#include <asm/time.h>
+#include <asm/rtc.h>
+
+#define DS1374_REG_TOD0		0x00
+#define DS1374_REG_TOD1		0x01
+#define DS1374_REG_TOD2		0x02
+#define DS1374_REG_TOD3		0x03
+#define DS1374_REG_WDALM0	0x04
+#define DS1374_REG_WDALM1	0x05
+#define DS1374_REG_WDALM2	0x06
+#define DS1374_REG_CR		0x07
+#define DS1374_REG_SR		0x08
+#define DS1374_REG_SR_OSF	0x80
+#define DS1374_REG_TCR		0x09
+
+#define	DS1374_DRV_NAME		"ds1374"
+
+static DECLARE_MUTEX(ds1374_mutex);
+
+static struct i2c_driver ds1374_driver;
+static struct i2c_client *save_client;
+
+static unsigned short ignore[] = { I2C_CLIENT_END };
+static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
+
+static struct i2c_client_address_data addr_data = {
+	.normal_i2c = normal_addr,
+	.normal_i2c_range = ignore,
+	.probe = ignore,
+	.probe_range = ignore,
+	.ignore = ignore,
+	.ignore_range = ignore,
+	.force = ignore,
+};
+
+static ulong ds1374_read_rtc(void)
+{
+	ulong time = 0;
+	int reg = DS1374_REG_WDALM0;
+
+	while (reg--) {
+		s32 tmp;
+		if ((tmp = i2c_smbus_read_byte_data(save_client, reg)) < 0) {
+			dev_warn(&save_client->dev,
+				 "can't read from rtc chip\n");
+			return 0;
+		}
+		time = (time << 8) | (tmp & 0xff);
+	}
+	return time;
+}
+
+static void ds1374_write_rtc(ulong time)
+{
+	int reg;
+
+	for (reg = DS1374_REG_TOD0; reg < DS1374_REG_WDALM0; reg++) {
+		if (i2c_smbus_write_byte_data(save_client, reg, time & 0xff)
+		    < 0) {
+			dev_warn(&save_client->dev,
+				 "can't write to rtc chip\n");
+			break;
+		}
+		time = time >> 8;
+	}
+}
+
+static void ds1374_check_rtc_status(void)
+{
+	s32 tmp;
+
+	tmp = i2c_smbus_read_byte_data(save_client, DS1374_REG_SR);
+	if (tmp < 0) {
+		dev_warn(&save_client->dev,
+			 "can't read status from rtc chip\n");
+		return;
+	}
+	if (tmp & DS1374_REG_SR_OSF) {
+		dev_warn(&save_client->dev,
+			 "oscillator discontinuity flagged, time unreliable\n");
+		tmp &= ~DS1374_REG_SR_OSF;
+		tmp = i2c_smbus_write_byte_data(save_client, DS1374_REG_SR,
+						tmp & 0xff);
+		if (tmp < 0)
+			dev_warn(&save_client->dev,
+				 "can't clear discontinuity notification\n");
+	}
+}
+
+ulong ds1374_get_rtc_time(void)
+{
+	ulong t1, t2;
+	int limit = 10;		/* arbitrary retry limit */
+
+	down(&ds1374_mutex);
+
+	/*
+	 * Since the reads are being performed one byte at a time using
+	 * the SMBus vs a 4-byte i2c transfer, there is a chance that a
+	 * carry will occur during the read. To detect this, 2 reads are
+	 * performed and compared.
+	 */
+	do {
+		t1 = ds1374_read_rtc();
+		t2 = ds1374_read_rtc();
+	} while (t1 != t2 && limit--);
+
+	up(&ds1374_mutex);
+
+	if (t1 != t2) {
+		dev_warn(&save_client->dev,
+			 "can't get consistent time from rtc chip\n");
+		t1 = 0;
+	}
+
+	return t1;
+}
+
+static void ds1374_set_tlet(ulong arg)
+{
+	ulong t1, t2;
+	int limit = 10;		/* arbitrary retry limit */
+
+	t1 = *(ulong *) arg;
+
+	down(&ds1374_mutex);
+
+	/*
+	 * Since the writes are being performed one byte at a time using
+	 * the SMBus vs a 4-byte i2c transfer, there is a chance that a
+	 * carry will occur during the write. To detect this, the write
+	 * value is read back and compared.
+	 */
+	do {
+		ds1374_write_rtc(t1);
+		t2 = ds1374_read_rtc();
+	} while (t1 != t2 && limit--);
+
+	up(&ds1374_mutex);
+
+	if (t1 != t2)
+		dev_warn(&save_client->dev,
+			 "can't confirm time set from rtc chip\n");
+}
+
+ulong new_time;
+
+DECLARE_TASKLET_DISABLED(ds1374_tasklet, ds1374_set_tlet, (ulong) & new_time);
+
+int ds1374_set_rtc_time(ulong nowtime)
+{
+	new_time = nowtime;
+
+	if (in_interrupt())
+		tasklet_schedule(&ds1374_tasklet);
+	else
+		ds1374_set_tlet((ulong) & new_time);
+
+	return 0;
+}
+
+/*
+ *****************************************************************************
+ *
+ *	Driver Interface
+ *
+ *****************************************************************************
+ */
+static int ds1374_probe(struct i2c_adapter *adap, int addr, int kind)
+{
+	struct i2c_client *client;
+	int rc;
+
+	client = kmalloc(sizeof(struct i2c_client), GFP_KERNEL);
+	if (!client)
+		return -ENOMEM;
+
+	memset(client, 0, sizeof(struct i2c_client));
+	strncpy(client->name, DS1374_DRV_NAME, I2C_NAME_SIZE);
+	client->flags = I2C_DF_NOTIFY;
+	client->addr = addr;
+	client->adapter = adap;
+	client->driver = &ds1374_driver;
+
+	if ((rc = i2c_attach_client(client)) != 0) {
+		kfree(client);
+		return rc;
+	}
+
+	save_client = client;
+
+	ds1374_check_rtc_status();
+
+	return 0;
+}
+
+static int ds1374_attach(struct i2c_adapter *adap)
+{
+	return i2c_probe(adap, &addr_data, ds1374_probe);
+}
+
+static int ds1374_detach(struct i2c_client *client)
+{
+	int rc;
+
+	if ((rc = i2c_detach_client(client)) == 0) {
+		kfree(i2c_get_clientdata(client));
+		tasklet_kill(&ds1374_tasklet);
+	}
+	return rc;
+}
+
+static struct i2c_driver ds1374_driver = {
+	.owner = THIS_MODULE,
+	.name = DS1374_DRV_NAME,
+	.id = I2C_DRIVERID_DS1374,
+	.flags = I2C_DF_NOTIFY,
+	.attach_adapter = ds1374_attach,
+	.detach_client = ds1374_detach,
+};
+
+static int __init ds1374_init(void)
+{
+	return i2c_add_driver(&ds1374_driver);
+}
+
+static void __exit ds1374_exit(void)
+{
+	i2c_del_driver(&ds1374_driver);
+}
+
+module_init(ds1374_init);
+module_exit(ds1374_exit);
+
+MODULE_AUTHOR("Randy Vinson <rvinson@mvista.com>");
+MODULE_DESCRIPTION("Maxim/Dallas DS1374 RTC I2C Client Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c-id.h b/include/linux/i2c-id.h
--- a/include/linux/i2c-id.h
+++ b/include/linux/i2c-id.h
@@ -108,6 +108,7 @@
 #define I2C_DRIVERID_TDA7313	62	/* TDA7313 audio processor	*/
 #define I2C_DRIVERID_MAX6900	63	/* MAX6900 real-time clock	*/
 #define I2C_DRIVERID_SAA7114H	64	/* video decoder		*/
+#define I2C_DRIVERID_DS1374	65	/* DS1374 real time clock	*/
 
 
 #define I2C_DRIVERID_EXP0	0xF0	/* experimental use id's	*/

Re: [PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Kumar Gala]

Randy,

I was planning on cloning the DS1337 driver once all of the latest  
patches for it got into Linus's tree.  Not sure if you looked at it,  
but when I was looking at RTC on 8349 the DS1337 seemed similar.  Not  
sure if we can unify the DS1337 driver such that can also support the  
DS1374.

(Also, I'll wait on acceptance of the driver before pushing the board  
code).

- kumar

On Jun 3, 2005, at 4:36 PM, Randy Vinson wrote:

> Greetings,
>   I've put together a small I2C client for the Maxim/Dallas DS1374 RTC
> chip and tested it on a Freescale MPC8349ADS board that uses the chip.
> The attached patch adds support for the chip itself and a follow-up  
> patch
> will add support to the Freescale board.
>
>   Please CC: me on any replies as I am not subscribed to the sensors  
> list.
>
> 		Thanks,
> 		Randy Vinson
>
>
> Add support for Maxim/Dallas DS1374 Real-Time Clock Chip
>
> This change adds support for the Maxim/Dallas DS1374 RTC chip. This  
> chip
> is an I2C-based RTC that maintains a simple 32-bit binary seconds count
> with battery backup support.
>
> Signed-off-by: Randy Vinson <rvinson@mvista.com>
>
> ---
> commit f95cc4d276cc332225bd823a2ae3be553c119990
> tree bd11f3262e29c13b6d56383415c634371394e913
> parent b56ae7a07ed0304bbc1dd61d3527dccdbcc467e9
> author Randy Vinson <rvinson@linuxbox.(none)> Fri, 03 Jun 2005  
> 13:50:59 -0700
> committer Randy Vinson <rvinson@linuxbox.(none)> Fri, 03 Jun 2005  
> 13:50:59 -0700
>
>  drivers/i2c/chips/Kconfig  |   11 ++
>  drivers/i2c/chips/Makefile |    1
>  drivers/i2c/chips/ds1374.c |  266  
> ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c-id.h     |    1
>  4 files changed, 279 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
> --- a/drivers/i2c/chips/Kconfig
> +++ b/drivers/i2c/chips/Kconfig
> @@ -376,6 +376,17 @@ config SENSORS_DS1337
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called ds1337.
>
> +config SENSORS_DS1374
> +	tristate "Maxim/Dallas Semiconductor DS1374 Real Time Clock"
> +	depends on I2C && EXPERIMENTAL
> +	select I2C_SENSOR
> +	help
> +	  If you say yes here you get support for Dallas Semiconductor
> +	  DS1374 real-time clock chips.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ds1374.
> +
>  config SENSORS_EEPROM
>  	tristate "EEPROM reader"
>  	depends on I2C && EXPERIMENTAL
> diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
> --- a/drivers/i2c/chips/Makefile
> +++ b/drivers/i2c/chips/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025
>  obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
>  obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
>  obj-$(CONFIG_SENSORS_DS1337)	+= ds1337.o
> +obj-$(CONFIG_SENSORS_DS1374)	+= ds1374.o
>  obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
>  obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o
>  obj-$(CONFIG_SENSORS_FSCHER)	+= fscher.o
> diff --git a/drivers/i2c/chips/ds1374.c b/drivers/i2c/chips/ds1374.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/i2c/chips/ds1374.c
> @@ -0,0 +1,266 @@
> +/*
> + * drivers/i2c/chips/ds1374.c
> + *
> + * I2C client/driver for the Maxim/Dallas DS1374 Real-Time Clock
> + *
> + * Author: Randy Vinson <rvinson@mvista.com>
> + *
> + * Based on the m41t00.c by Mark Greer <mgreer@mvista.com>
> + *
> + * 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.
> + */
> +/*
> + * This i2c client/driver wedges between the drivers/char/genrtc.c RTC
> + * interface and the SMBus interface of the i2c subsystem.
> + * It would be more efficient to use i2c msgs/i2c_transfer directly  
> but, as
> + * recommened in .../Documentation/i2c/writing-clients section
> + * "Sending and receiving", using SMBus level communication is  
> preferred.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/rtc.h>
> +#include <linux/bcd.h>
> +
> +#include <asm/time.h>
> +#include <asm/rtc.h>
> +
> +#define DS1374_REG_TOD0		0x00
> +#define DS1374_REG_TOD1		0x01
> +#define DS1374_REG_TOD2		0x02
> +#define DS1374_REG_TOD3		0x03
> +#define DS1374_REG_WDALM0	0x04
> +#define DS1374_REG_WDALM1	0x05
> +#define DS1374_REG_WDALM2	0x06
> +#define DS1374_REG_CR		0x07
> +#define DS1374_REG_SR		0x08
> +#define DS1374_REG_SR_OSF	0x80
> +#define DS1374_REG_TCR		0x09
> +
> +#define	DS1374_DRV_NAME		"ds1374"
> +
> +static DECLARE_MUTEX(ds1374_mutex);
> +
> +static struct i2c_driver ds1374_driver;
> +static struct i2c_client *save_client;
> +
> +static unsigned short ignore[] = { I2C_CLIENT_END };
> +static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
> +
> +static struct i2c_client_address_data addr_data = {
> +	.normal_i2c = normal_addr,
> +	.normal_i2c_range = ignore,
> +	.probe = ignore,
> +	.probe_range = ignore,
> +	.ignore = ignore,
> +	.ignore_range = ignore,
> +	.force = ignore,
> +};
> +
> +static ulong ds1374_read_rtc(void)
> +{
> +	ulong time = 0;
> +	int reg = DS1374_REG_WDALM0;
> +
> +	while (reg--) {
> +		s32 tmp;
> +		if ((tmp = i2c_smbus_read_byte_data(save_client, reg)) < 0) {
> +			dev_warn(&save_client->dev,
> +				 "can't read from rtc chip\n");
> +			return 0;
> +		}
> +		time = (time << 8) | (tmp & 0xff);
> +	}
> +	return time;
> +}
> +
> +static void ds1374_write_rtc(ulong time)
> +{
> +	int reg;
> +
> +	for (reg = DS1374_REG_TOD0; reg < DS1374_REG_WDALM0; reg++) {
> +		if (i2c_smbus_write_byte_data(save_client, reg, time & 0xff)
> +		    < 0) {
> +			dev_warn(&save_client->dev,
> +				 "can't write to rtc chip\n");
> +			break;
> +		}
> +		time = time >> 8;
> +	}
> +}
> +
> +static void ds1374_check_rtc_status(void)
> +{
> +	s32 tmp;
> +
> +	tmp = i2c_smbus_read_byte_data(save_client, DS1374_REG_SR);
> +	if (tmp < 0) {
> +		dev_warn(&save_client->dev,
> +			 "can't read status from rtc chip\n");
> +		return;
> +	}
> +	if (tmp & DS1374_REG_SR_OSF) {
> +		dev_warn(&save_client->dev,
> +			 "oscillator discontinuity flagged, time unreliable\n");
> +		tmp &= ~DS1374_REG_SR_OSF;
> +		tmp = i2c_smbus_write_byte_data(save_client, DS1374_REG_SR,
> +						tmp & 0xff);
> +		if (tmp < 0)
> +			dev_warn(&save_client->dev,
> +				 "can't clear discontinuity notification\n");
> +	}
> +}
> +
> +ulong ds1374_get_rtc_time(void)
> +{
> +	ulong t1, t2;
> +	int limit = 10;		/* arbitrary retry limit */
> +
> +	down(&ds1374_mutex);
> +
> +	/*
> +	 * Since the reads are being performed one byte at a time using
> +	 * the SMBus vs a 4-byte i2c transfer, there is a chance that a
> +	 * carry will occur during the read. To detect this, 2 reads are
> +	 * performed and compared.
> +	 */
> +	do {
> +		t1 = ds1374_read_rtc();
> +		t2 = ds1374_read_rtc();
> +	} while (t1 != t2 && limit--);
> +
> +	up(&ds1374_mutex);
> +
> +	if (t1 != t2) {
> +		dev_warn(&save_client->dev,
> +			 "can't get consistent time from rtc chip\n");
> +		t1 = 0;
> +	}
> +
> +	return t1;
> +}
> +
> +static void ds1374_set_tlet(ulong arg)
> +{
> +	ulong t1, t2;
> +	int limit = 10;		/* arbitrary retry limit */
> +
> +	t1 = *(ulong *) arg;
> +
> +	down(&ds1374_mutex);
> +
> +	/*
> +	 * Since the writes are being performed one byte at a time using
> +	 * the SMBus vs a 4-byte i2c transfer, there is a chance that a
> +	 * carry will occur during the write. To detect this, the write
> +	 * value is read back and compared.
> +	 */
> +	do {
> +		ds1374_write_rtc(t1);
> +		t2 = ds1374_read_rtc();
> +	} while (t1 != t2 && limit--);
> +
> +	up(&ds1374_mutex);
> +
> +	if (t1 != t2)
> +		dev_warn(&save_client->dev,
> +			 "can't confirm time set from rtc chip\n");
> +}
> +
> +ulong new_time;
> +
> +DECLARE_TASKLET_DISABLED(ds1374_tasklet, ds1374_set_tlet, (ulong) &  
> new_time);
> +
> +int ds1374_set_rtc_time(ulong nowtime)
> +{
> +	new_time = nowtime;
> +
> +	if (in_interrupt())
> +		tasklet_schedule(&ds1374_tasklet);
> +	else
> +		ds1374_set_tlet((ulong) & new_time);
> +
> +	return 0;
> +}
> +
> +/*
> +  
> *********************************************************************** 
> ******
> + *
> + *	Driver Interface
> + *
> +  
> *********************************************************************** 
> ******
> + */
> +static int ds1374_probe(struct i2c_adapter *adap, int addr, int kind)
> +{
> +	struct i2c_client *client;
> +	int rc;
> +
> +	client = kmalloc(sizeof(struct i2c_client), GFP_KERNEL);
> +	if (!client)
> +		return -ENOMEM;
> +
> +	memset(client, 0, sizeof(struct i2c_client));
> +	strncpy(client->name, DS1374_DRV_NAME, I2C_NAME_SIZE);
> +	client->flags = I2C_DF_NOTIFY;
> +	client->addr = addr;
> +	client->adapter = adap;
> +	client->driver = &ds1374_driver;
> +
> +	if ((rc = i2c_attach_client(client)) != 0) {
> +		kfree(client);
> +		return rc;
> +	}
> +
> +	save_client = client;
> +
> +	ds1374_check_rtc_status();
> +
> +	return 0;
> +}
> +
> +static int ds1374_attach(struct i2c_adapter *adap)
> +{
> +	return i2c_probe(adap, &addr_data, ds1374_probe);
> +}
> +
> +static int ds1374_detach(struct i2c_client *client)
> +{
> +	int rc;
> +
> +	if ((rc = i2c_detach_client(client)) == 0) {
> +		kfree(i2c_get_clientdata(client));
> +		tasklet_kill(&ds1374_tasklet);
> +	}
> +	return rc;
> +}
> +
> +static struct i2c_driver ds1374_driver = {
> +	.owner = THIS_MODULE,
> +	.name = DS1374_DRV_NAME,
> +	.id = I2C_DRIVERID_DS1374,
> +	.flags = I2C_DF_NOTIFY,
> +	.attach_adapter = ds1374_attach,
> +	.detach_client = ds1374_detach,
> +};
> +
> +static int __init ds1374_init(void)
> +{
> +	return i2c_add_driver(&ds1374_driver);
> +}
> +
> +static void __exit ds1374_exit(void)
> +{
> +	i2c_del_driver(&ds1374_driver);
> +}
> +
> +module_init(ds1374_init);
> +module_exit(ds1374_exit);
> +
> +MODULE_AUTHOR("Randy Vinson <rvinson@mvista.com>");
> +MODULE_DESCRIPTION("Maxim/Dallas DS1374 RTC I2C Client Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/i2c-id.h b/include/linux/i2c-id.h
> --- a/include/linux/i2c-id.h
> +++ b/include/linux/i2c-id.h
> @@ -108,6 +108,7 @@
>  #define I2C_DRIVERID_TDA7313	62	/* TDA7313 audio processor	*/
>  #define I2C_DRIVERID_MAX6900	63	/* MAX6900 real-time clock	*/
>  #define I2C_DRIVERID_SAA7114H	64	/* video decoder		*/
> +#define I2C_DRIVERID_DS1374	65	/* DS1374 real time clock	*/
>
>
>  #define I2C_DRIVERID_EXP0	0xF0	/* experimental use id's	*/

[PATCH 2/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Randy Vinson]

Greetings,
  This is part 2 of the patch to add support for the DS1374 I2C RTC chip
from Maxim/Dallas.


		Randy Vinson



Add support for the I2C Real-Time Clock on the MPC8349ADS board.

This change provides support for the DS1374 Real-Time Clock chip present
on the MPC8349ADS board. It depends on a previous patch which adds I2C
support for the DS1374.

Signed-off-by: Randy Vinson <rvinson@mvista.com>

---
commit 07fb1bd27347f7f0df1309c40d09a2f1a9a98731
tree b3ac3ac93679372aa5111268b623c5dda4a5039e
parent f95cc4d276cc332225bd823a2ae3be553c119990
author Randy Vinson <rvinson@linuxbox.(none)> Fri, 03 Jun 2005 13:57:46 -0700
committer Randy Vinson <rvinson@linuxbox.(none)> Fri, 03 Jun 2005 13:57:46 -0700

 arch/ppc/platforms/83xx/mpc834x_sys.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/arch/ppc/platforms/83xx/mpc834x_sys.c b/arch/ppc/platforms/83xx/mpc834x_sys.c
--- a/arch/ppc/platforms/83xx/mpc834x_sys.c
+++ b/arch/ppc/platforms/83xx/mpc834x_sys.c
@@ -227,6 +227,26 @@ mpc834x_sys_init_IRQ(void)
 	ipic_set_default_priority();
 }
 
+#if defined(CONFIG_I2C_MPC) && defined(CONFIG_SENSORS_DS1374)
+extern ulong	ds1374_get_rtc_time(void);
+extern int	ds1374_set_rtc_time(ulong);
+
+static int __init
+mpc834x_rtc_hookup(void)
+{
+	struct timespec	tv;
+
+	ppc_md.get_rtc_time = ds1374_get_rtc_time;
+	ppc_md.set_rtc_time = ds1374_set_rtc_time;
+
+	tv.tv_nsec = 0;
+	tv.tv_sec = (ppc_md.get_rtc_time)();
+	do_settimeofday(&tv);
+
+	return 0;
+}
+late_initcall(mpc834x_rtc_hookup);
+#endif
 static __inline__ void
 mpc834x_sys_set_bat(void)
 {

Re: [PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Eugene Surovegin]

On Fri, Jun 03, 2005 at 02:36:06PM -0700, Randy Vinson wrote:
> Greetings,
>  I've put together a small I2C client for the Maxim/Dallas DS1374 RTC
> chip and tested it on a Freescale MPC8349ADS board that uses the chip.
> The attached patch adds support for the chip itself and a follow-up patch
> will add support to the Freescale board.

[snip]

> +	down(&ds1374_mutex);
> +
> +	/*
> +	 * Since the reads are being performed one byte at a time using
> +	 * the SMBus vs a 4-byte i2c transfer, there is a chance that a
> +	 * carry will occur during the read. To detect this, 2 reads are
> +	 * performed and compared.
> +	 */
> +	do {
> +		t1 = ds1374_read_rtc();
> +		t2 = ds1374_read_rtc();
> +	} while (t1 != t2 && limit--);

I wonder, why you chose to use those 1-byte SMBus transfers instead of 
i2c transfer.

I wrote similar DS1374 driver some time ago which used those transfers 
and they worked just fine.

-- 
Eugene

Re: [PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Randy Vinson]

Kumar Gala wrote:
> Randy,
> 
> I was planning on cloning the DS1337 driver once all of the latest  
> patches for it got into Linus's tree. 

I haven't seen any patches for the DS1337, but I admit that I wasn't 
looking very hard.

  Not sure if you looked at it,
> but when I was looking at RTC on 8349 the DS1337 seemed similar.  Not  
> sure if we can unify the DS1337 driver such that can also support the  
> DS1374.

I looked at the DS1337 driver, but that chip uses individual registers 
for the various time bits, whereas the DS1374 uses a 32-bit seconds 
count spread across 4 registers. I wasn't convinced that I could find a 
reliable way to differenciate between the 2 types at run time since they 
use the same I2C address (0x68). So, I choose to create a new driver to 
keep it simple. If there is a reliable way to handle both types in the 
same driver, I'll drop my DS1374-specific driver.

> 
> (Also, I'll wait on acceptance of the driver before pushing the board  
> code).

Agreed.

		Randy Vinson

Re: [PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Randy Vinson]

Eugene Surovegin wrote:
[snip]
> 
> I wonder, why you chose to use those 1-byte SMBus transfers instead of 
> i2c transfer.

I was simply following the guidelines in 
Documentation/i2c/writing-clients as noted in the driver header. This 
note was in the driver I used as my base, so I just followed along.

> 
> I wrote similar DS1374 driver some time ago which used those transfers 
> and they worked just fine.

I checked http://secure.netroedge.com/~lm78/supported.html, 
http://secure.netroedge.com/~lm78/newdrivers.html and looked in the 
lm_sensors-2.9.1 tarball before I started and didn't see a driver for 
the DS1374 listed. That's why I threw mine together. Maybe I missed it.

I would have used I2C transfers myself, but was simply following what I 
thought were current practices. Since this is my first I2C client, I 
just blindly followed the documentation :) Oh well, wouldn't be the 
first time I wandered down the wrong path.

		Randy Vinson

Re: [lm-sensors] Re: [PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Jean Delvare]

Hi Randy and all,

Sorry for the delay.

[Eugene Surovegin]
> I wonder, why you chose to use those 1-byte SMBus transfers instead
> of  i2c transfer.

[Randy Vinson]
> I was simply following the guidelines in 
> Documentation/i2c/writing-clients as noted in the driver header. This 
> note was in the driver I used as my base, so I just followed along.

The document file says:

"If you can choose between plain i2c communication and SMBus level
communication, please use the last. All adapters understand SMBus level
commands, but only some of them understand plain i2c!"

The "if you can choose" is important. It doesn't suggest that you should
use a less efficient way, but that you use the SMBus API when the I2C
transfer you want to use happens to exist in the SMBus API.

Even when sticking to SMBus commands, it is quite frequent that there
are different ways to retrieve the same information, of varying
availability and speed. One good example of this are EEPROMs. EEPROMs
are I2C devices which can be accessed using SMBus commands. You can read
bytes one by one (SMBus Read Byte Data command), or continuously as
different reads (first one SMBus Write Byte, then many SMBus Read Byte),
or continuously as a single read (I2C Block Read, supported by some
SMBus controllers). If you look at the eeprom.c drivers, you'll see that
the second and the third method are both implemented. The advantage is
that:

1* The SMBus Write/Read Byte method is supported by almost all SMBus and
all I2C masters.

2* The I2C Block Read method is twice as fast, and works on all true I2C
masters and a few SMBus masters.

So we get the best from each hardware configuration.

 As I read the DS1374 datasheet, it appears to support all three modes
(it's a true I2C device) so you should be able to do something similar
to what is done in the eeprom driver.

> I checked http://secure.netroedge.com/~lm78/supported.html, 
> http://secure.netroedge.com/~lm78/newdrivers.html and looked in the 
> lm_sensors-2.9.1 tarball before I started and didn't see a driver for 
> the DS1374 listed. That's why I threw mine together. Maybe I missed
> it.

The DS1374 isn't a hardware monitoring driver, so it doesn't have to be
listed here (although we happen to list a few miscellaneous drivers).

-- 
Jean Delvare

Re: [lm-sensors] [PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Jean Delvare]

Hi Randy,

Here are a few random comments on your code, sorry for the delay.

> +static struct i2c_client_address_data addr_data = {
> +	.normal_i2c = normal_addr,
> +	.normal_i2c_range = ignore,
> +	.probe = ignore,
> +	.probe_range = ignore,
> +	.ignore = ignore,
> +	.ignore_range = ignore,
> +	.force = ignore,
> +};

Ranges are gone in -mm, please drop them. You should always base your
patches on -mm BTW, as this is what we will attempt to apply it against.

> +static ulong ds1374_read_rtc(void)
> +{
> +	ulong time = 0;
> +	int reg = DS1374_REG_WDALM0;
> +
> +	while (reg--) {
> +		s32 tmp;
> +		if ((tmp = i2c_smbus_read_byte_data(save_client, reg)) < 0) {
> +			dev_warn(&save_client->dev,
> +				 "can't read from rtc chip\n");
> +			return 0;
> +		}
> +		time = (time << 8) | (tmp & 0xff);

& 0xff isn't needed.

> +ulong ds1374_get_rtc_time(void)
> +{
> +	ulong t1, t2;
> +	int limit = 10;		/* arbitrary retry limit */
> +
> +	down(&ds1374_mutex);
> +
> +	/*
> +	 * Since the reads are being performed one byte at a time using
> +	 * the SMBus vs a 4-byte i2c transfer, there is a chance that a
> +	 * carry will occur during the read. To detect this, 2 reads are
> +	 * performed and compared.
> +	 */
> +	do {
> +		t1 = ds1374_read_rtc();
> +		t2 = ds1374_read_rtc();
> +	} while (t1 != t2 && limit--);
> +
> +	up(&ds1374_mutex);
> +
> +	if (t1 != t2) {
> +		dev_warn(&save_client->dev,
> +			 "can't get consistent time from rtc chip\n");
> +		t1 = 0;
> +	}
> +
> +	return t1;
> +}
> +
> +static void ds1374_set_tlet(ulong arg)
> +{
> +	ulong t1, t2;
> +	int limit = 10;		/* arbitrary retry limit */
> +
> +	t1 = *(ulong *) arg;
> +
> +	down(&ds1374_mutex);
> +
> +	/*
> +	 * Since the writes are being performed one byte at a time using
> +	 * the SMBus vs a 4-byte i2c transfer, there is a chance that a
> +	 * carry will occur during the write. To detect this, the write
> +	 * value is read back and compared.
> +	 */
> +	do {
> +		ds1374_write_rtc(t1);
> +		t2 = ds1374_read_rtc();
> +	} while (t1 != t2 && limit--);
> +
> +	up(&ds1374_mutex);
> +
> +	if (t1 != t2)
> +		dev_warn(&save_client->dev,
> +			 "can't confirm time set from rtc chip\n");
> +}

The above two functions suggest that a continuous read from the chip
would be very welcome (see my previous message). I even doubt anyone
would connect this kind of RTC to a bus NOT supporting I2C block reads,
because then other models would make more sense. So I'd suggest you use
I2C block reads (and SMBus block writes) here. If anyone really needs a
different access method, they can add it afterwards. Doing so will make
your driver much faster, smaller and more simple.

Which bus are you using this chip BTW, some bit-banging I2C?

Oh, and the ulong* vs. ulong cast is really ugly. Can't it be done
differently?

> +ulong new_time;

Shouldn't it be static?

> +static int ds1374_probe(struct i2c_adapter *adap, int addr, int kind)
> +{
> +	struct i2c_client *client;
> +	int rc;

Here you should check whether the adapter supports the I2C/SMBus
commands you are about to throw upon it. See how the other i2c chip
drivers do.

> +
> +	client = kmalloc(sizeof(struct i2c_client), GFP_KERNEL);
> +	if (!client)
> +		return -ENOMEM;
> +
> +	memset(client, 0, sizeof(struct i2c_client));
> +	strncpy(client->name, DS1374_DRV_NAME, I2C_NAME_SIZE);
> +	client->flags = I2C_DF_NOTIFY;

This is a *driver* flag, not meaningful nor suitable for a client
structure. Where did you copy that from?

> +	client->addr = addr;
> +	client->adapter = adap;
> +	client->driver = &ds1374_driver;

You should have some kind of chip detection at this point. Failing to do
so, you may catch any chip at this address and misdrive it. Detection of
these chips is always a bit tricky, but you could do the following:

* Check the 6 zero bits of status register.

* If there are other register-based assertions, use them (I have no time
to read the full datasheet).

* See what happens when you try to read past the register count (0x0A
and above) in either individual byte or block mode. Sometimes it wraps,
sometimes it returns an error, sometimes in repeats the last register,
depends on the chip. This is a trick, but very useful in this situation.

> +static int ds1374_detach(struct i2c_client *client)
> +{
> +	int rc;
> +
> +	if ((rc = i2c_detach_client(client)) == 0) {
> +		kfree(i2c_get_clientdata(client));
> +		tasklet_kill(&ds1374_tasklet);
> +	}

You should kill the tasklet first, else it might try to use the client
after you freed it, right?

> +static struct i2c_driver ds1374_driver = {
> +	.owner = THIS_MODULE,
> +	.name = DS1374_DRV_NAME,
> +	.id = I2C_DRIVERID_DS1374,

You don't need this ID, so you should just omit it. Also, it is
recommended to align structure declarations on the equal sign (using
tabs, not spaces), for a better readability.

Other comments:

1* Your driver construction makes it impossible to support more than one
device.

2* You seem to have two public functions in this driver, yet there is no
symbol export of these?

-- 
Jean Delvare

Re: [lm-sensors] Re: [PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Mark A. Greer]

Hi Jean,

Jean Delvare wrote:

>Hi Randy and all,
>
>Sorry for the delay.
>
>[Eugene Surovegin]
>  
>
>>I wonder, why you chose to use those 1-byte SMBus transfers instead
>>of  i2c transfer.
>>    
>>
>
>[Randy Vinson]
>  
>
>>I was simply following the guidelines in 
>>Documentation/i2c/writing-clients as noted in the driver header. This 
>>note was in the driver I used as my base, so I just followed along.
>>    
>>
>
>The document file says:
>
>"If you can choose between plain i2c communication and SMBus level
>communication, please use the last. All adapters understand SMBus level
>commands, but only some of them understand plain i2c!"
>
>The "if you can choose" is important. It doesn't suggest that you should
>use a less efficient way, but that you use the SMBus API when the I2C
>transfer you want to use happens to exist in the SMBus API.
>
>Even when sticking to SMBus commands, it is quite frequent that there
>are different ways to retrieve the same information, of varying
>availability and speed. One good example of this are EEPROMs. EEPROMs
>are I2C devices which can be accessed using SMBus commands. You can read
>bytes one by one (SMBus Read Byte Data command), or continuously as
>different reads (first one SMBus Write Byte, then many SMBus Read Byte),
>or continuously as a single read (I2C Block Read, supported by some
>SMBus controllers). If you look at the eeprom.c drivers, you'll see that
>the second and the third method are both implemented. The advantage is
>that:
>

I interpreted the doc the same way Randy did when I did the m41t00 chip 
code.  Also, Mark Studebaker seemed to support that interpretation in 
this email: http://archives.andrew.net.au/lm-sensors/msg29325.html

I suppose the best thing to do is check if the adapter is a true i2c 
ctlr (something like: i2c_check_functionality(adapter, I2C_FUNC_I2C)) 
and base the cmds used on that.

We can do something similar with I2C_FUNC_SMBUS_READ_I2C_BLOCK & 
I2C_FUNC_SMBUS_WRITE_BLOCK_DATA to check if we can use the smbus block 
cmds too, correct?

Mark

Re: [lm-sensors] Re: [PATCH 1/2] Add support for Maxim/Dallas DS1374 Real-Time Clock Chip

[Jean Delvare]

Hi Mark,

> I interpreted the doc the same way Randy did when I did the m41t00
> chip  code.  Also, Mark Studebaker seemed to support that
> interpretation in  this email:
> http://archives.andrew.net.au/lm-sensors/msg29325.html

I am not discussing the fast that using basic SMBus commands increases
the number of system the driver will be usable on. This is quite
obviously true, and I am fine with using this when there is no
performance drawback.

However, in the case of the DS1374, reading the registers as a block
would make the driver *much* faster and more simple. This has to be
considered, especially if the DS1374 chip is found on a limited number
of systems.

If you think the documentation should be updated to reflect that better,
well, patches are welcome ;)

> I suppose the best thing to do is check if the adapter is a true i2c 
> ctlr (something like: i2c_check_functionality(adapter, I2C_FUNC_I2C)) 
> and base the cmds used on that.
> 
> We can do something similar with I2C_FUNC_SMBUS_READ_I2C_BLOCK & 
> I2C_FUNC_SMBUS_WRITE_BLOCK_DATA to check if we can use the smbus
> block cmds too, correct?

There is actually no benefit in checking for true I2C, since the
commands you would then build would be exactly I2C_FUNC_SMBUS_READ_I2C
and I2C_FUNC_SMBUS_WRITE_BLOCK_DATA. Better check for these directly, as
some non-I2C masters will support them (especially the second, the first
one is only rarely found). If both functions are supported, go with
this. If not, either fallback to the current code, or drop this code and
state that this isn't supported (I'd do that). If it happens that this
support is ever needed, it can be added back at that time - but that
time may never actually come.

Thanks,
-- 
Jean Delvare