On Sun, 10 Apr 2005 12:47:42 -0400 Derek Cheung wrote:
| Enclosed please find the updated patch that incorporates changes for all
| the comments I received.
(yes, almost all)
| The volatile declaration in the m528xsim.h is needed because the
| declaration refers to the ColdFire 5282 register mapping. The volatile
| declaration is actually not needed in my I2C driver but someone may
| include the m528xsim.h file in his/her applications and we need to force
| the compiler not to do any optimization on the register mapping.
Did you also send it to the I2C mailing list like Greg asked you to do?
More comments:
diffstat summary, like so, would be nice:
drivers/i2c/busses/Kconfig | 10
drivers/i2c/busses/Makefile | 2
drivers/i2c/busses/i2c-mcf5282.c | 419 +++++++++++++++++++++++++++++++++++++++
drivers/i2c/busses/i2c-mcf5282.h | 46 ++++
include/asm-m68knommu/m528xsim.h | 42 +++
5 files changed, 519 insertions(+)
+ int i, len, rc = 0;
+ u8 rxData, tempRxData[2];
Use tabs, not spaces. Happens other places too.
+ switch (size) {
+ case I2C_SMBUS_QUICK:
Don't need to indent case statements... (repeating myself).
+ case I2C_SMBUS_PROC_CALL:
+ /* dev_info(&adap->dev, "size =
+ I2C_SMBUS_PROC_CALL \n"); */
No break needed here ?
+ case I2C_SMBUS_WORD_DATA:
+ if (rc < 0)
+ return -1;
+ else
+ return 0;
There are several of these. Why not just return rc ?
Kconfig says that the module (if selected) will be called
i2c-mcf5282lite, but Makefile builds
+obj-$(CONFIG_I2C_MCF5282LITE) += i2c-mcf5282.o
(i.e., no "lite").
---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
