Messages in this thread | | | Date | Sun, 10 Apr 2005 20:32:44 -0700 | From | "Randy.Dunlap" <> | Subject | Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU |
| |
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |