On Fri, Apr 11, 2008 at 02:24:55PM +0200, Wolfram Sang wrote:
> I'll just review myself to ask for some comments where I think some
> more opinions would be good. I also tried to contact David Brownell as
> the original author directly last week, but all my mails got blocked;
> I hope we can negotiate through this list.
Hmm, usually, this should work.
> > + * at24.c - handle most I2C EEPROMs
> Maybe rename the driver? at24 is vendor-specific. 24xx? 24cxx?
> eeprom-ng? I'd go for 24xx.
Don't use to many "xx" characters. Last time someone did that, he was
blocked by rmk's spam filter :-) Maybe "i2c-eeprom"?
> > + /* Lock protects against activities from other Linux tasks,
> > + * but not from changes by other I2C masters.
> > + */
Multi-line comments are usually written like
/*
* foo
*/
> To support the X24645, it would be necessary to raise AT24_MAX_CLIENTS
> to 32 (what a beast!). Then again, most eeproms will just need one
> client, so this would cause quite some overhead in most use-cases.
> Maybe it pays off to hande this dynamically?
As eeproms are normally slow things anyway, would it be a big
performance impact?
> > +static struct i2c_client *at24_ee_address(
> > + struct at24_data *at24,
> > + u16 *addr,
> > + unsigned *offset
Minor nit: the kernel usually doesn't aling the variables vertically; if
you add more and longer data types, it might be necessary to move around
all other lines, which clutters patches. So I'd write this as
struct at24_data *at24,
u16 *addr,
unsigned *offset
Robert
--
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Hannoversche Str. 2, 31134 Hildesheim, Germany
Phone: +49-5121-206917-0 | Fax: +49-5121-206917-9
_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c
