On Wed, 2010-09-01 at 11:26 +0900, Masayuki Ohtak wrote: > I2C driver of Topcliff PCH > +++ b/drivers/i2c/busses/i2c-pch.c [] > +static void pch_init(struct i2c_algo_pch_data *adap) [] > + if (pch_i2c_speed == FAST_MODE_CLK) { > + reg_value |= FAST_MODE_EN; > + dev_dbg(adap->pch_adapter.dev.parent, "Fast mode enabled\n"); > + } Just a suggestion below, ignore it at your pleasure... These dev_<level> calls might be easier to read if you had some #defines like: #define pch_dbg(adap, fmt, arg...) \ dev_dbg(adap->pch_adapter.dev.parent, fmt, ##arg) #define pch_err(adap, fmt, arg...) \ dev_err(adap->pch_adapter.dev.parent, fmt, ##arg) #define pch_info(adap, fmt, arg...) \ dev_info(adap->pch_adapter.dev.parent, fmt, ##arg) then the last dev_dbg becomes: pch_dbg(adap, "Fast mode enabled\n"); Many modules use similar wrapper #defines. > + dev_dbg(adap->pch_adapter.dev.parent, > + "%s: I2CCTL=%x pch_i2cbc=%x pch_i2ctmr=%x Enable interrupts\n", > + __func__, ioread32(p + PCH_I2CCTL), > + pch_i2cbc, pch_i2ctmr); pch_dbg(adap, etc...) etc. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html