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

[James Chapman <jchapman@katalix.com>]

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