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    [W:0.023 / U:0.220 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site