i2c chips: ds1337 1/2

Patch for ds1337 i2c driver:

Add code to handle case where board firmware does not start the RTC.

This patch was contributed by Michael Burian.

James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

Add code to handle case where board firmware does not start the RTC.

This patch was contributed by Michael Burian.

Index: linux-2.6.14/drivers/i2c/chips/ds1337.c
===================================================================
@@ -149,12 +149,25 @@
 	u8 buf[8];
 	u8 val;
 	struct i2c_msg msg[1];
+	u8 status, control;
+	int i;
 
 	if (!dt) {
 		dev_dbg(&client->dev, "%s: EINVAL: dt=NULL\n", __FUNCTION__);
 		return -EINVAL;
 	}
 
+	status  = i2c_smbus_read_byte_data(client, DS1337_REG_STATUS);
+	control = i2c_smbus_read_byte_data(client, DS1337_REG_CONTROL);
+	
+	/* On some boards, the RTC isn't configured by boot firmware. 
+	 * Handle that case by starting/configuring the RTC now.
+	 */
+	if ((status & 0x80) || (control & 0x80))
+		/* Initialize all, including STATUS and CONTROL to zero */
+		for (i = 0; i < 16; i++)
+			i2c_smbus_write_byte_data(client, i, 0);
+
 	dev_dbg(&client->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,

Hi James, Michael, all,

I understand the idea, but I don't like the implementation. Why did you add the initialization code to ds1337_set_datetime, rather than to ds1337_init_client where is seems to belong?

Also, the initialization loop is way less efficient than it could be, given the fact that the DS1337 autoincrements its address register on write. You could at least use i2c_smbus_write_byte (instead of i2c_smbus_write_byte_data), but even more efficient would be a block transfer, like the ds1337_set_datetime function use.

Care to respin a patch?

Thanks,

Jean Delvare

Michael originally put the code in init_client. I moved it because I thought some might not want the RTC started at init time, only when userspace explicitely set time. It's no big deal to move it again.

I agree. I missed that.

Michael?

James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development