[PATCH: 2.6.14] i2c chips: ds1337 1/2

[PATCH: 2.6.14] i2c chips: ds1337 1/2

[James Chapman]

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

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.

Signed-off-by: Michael Burian <dynmail1@gassner-waagen.at>
Signed-off-by: James Chapman <jchapman@katalix.com>

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




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

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

This patch was contributed by Michael Burian.

Signed-off-by: Michael Burian <dynmail1@gassner-waagen.at>
Signed-off-by: James Chapman <jchapman@katalix.com>

Index: linux-2.6.14/drivers/i2c/chips/ds1337.c
===================================================================
--- linux-2.6.14.orig/drivers/i2c/chips/ds1337.c	2005-11-02 19:30:17.000000000 +0000
+++ linux-2.6.14/drivers/i2c/chips/ds1337.c	2005-11-03 20:14:58.981735046 +0000
@@ -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,

Re: [PATCH: 2.6.14] i2c chips: ds1337 1/2

[Jean Delvare]

Hi James, Michael, all,

> Patch for ds1337 i2c driver:
> 
> Add code to handle case where board firmware does not start the
> RTC.

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

Re: [PATCH: 2.6.14] i2c chips: ds1337 1/2

[James Chapman]

Jean Delvare wrote:
> Hi James, Michael, all,
> 
> 
>>Patch for ds1337 i2c driver:
>>
>>Add code to handle case where board firmware does not start the
>>RTC.
> 
> 
> 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?

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.

> 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.

I agree. I missed that.

> Care to respin a patch?

Michael?

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