Dear ksi@koi8.net,
In message Pine.LNX.4.64ksi.0902121420120.21067@home-gw.koi8.net you wrote: ...
+/* I2C registers field definitions */ +/* --------------------------------*/ +/* I2C_CONTROL (R/W) */
Incorrect multi-line comment style. Please fix.
+#define SM501_CHECK_NACK() \
- do {\
if (tmp & (SM501_I2C_NACK | SM501_I2C_BUS_ERROR)) {\tmp = read_i2c_reg(SM501_I2C_CONTROL) & SM501_I2C_SPEED;\write_i2c_reg(SM501_I2C_CONTROL, tmp);\return(1);\}\- } while (0)
Macros with magic side effects (here on the variable "tmp" are stronly deprecated. Please fix this.
+static __inline__ void write_i2c_reg(unsigned long offset, u_int8_t data) +{
- writeb(data, sm501_iomem_base + SM501_I2C + offset);
- __asm__("sync;isync;msync");
Are you sure the "sync;isync;msync" is needed? The accessor functions should make sure this is not necessary.
Also, instead of register offsets ("...base + SM501_I2C") please use a proper data structure.
- do {
stat = read_i2c_reg(SM501_I2C_STATUS);if (stat & mask) {return(stat);}
No curly braces for single line statements. Same goes everywhere esle, too.
And "return" is not a function - please omit the parens (here and elsewhere).
- while ((read_i2c_reg(SM501_I2C_STATUS) & SM501_I2C_BUS_BUSY) && timeout--) {
...
diff -purN u-boot-i2c.orig/include/sm501-regs.h u-boot-i2c/include/sm501-regs.h --- u-boot-i2c.orig/include/sm501-regs.h 1969-12-31 16:00:00.000000000 -0800 +++ u-boot-i2c/include/sm501-regs.h 2009-02-12 10:46:00.000000000 -0800 @@ -0,0 +1,394 @@ +/* sm501-regs.h
Incorrect multiline comment style.
...
+#define SM501_GPIO31_0_CONTROL (0x000008) +#define SM501_GPIO63_32_CONTROL (0x00000C) +#define SM501_DRAM_CONTROL (0x000010)
+/* command list */ +#define SM501_ARBTRTN_CONTROL (0x000014)
+/* command list */ +#define SM501_COMMAND_LIST_STATUS (0x000024)
+/* interrupt debug */ +#define SM501_RAW_IRQ_STATUS (0x000028) +#define SM501_RAW_IRQ_CLEAR (0x000028) +#define SM501_IRQ_STATUS (0x00002C) +#define SM501_IRQ_MASK (0x000030) +#define SM501_DEBUG_CONTROL (0x000034)
Please do not use register offsets, define a C structure instead.
Best regards,
Wolfgang Denk