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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/