lkml.org 
[lkml]   [2005]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/
\
 
 \ /
  Last update: 2005-04-11 05:36    [from the cache]
©2003-2011 Jasper Spaans