Hi Uwe, On Wed, 2015-01-21 at 09:20 +0100, Uwe Kleine-König wrote: > Hello, > > On Wed, Jan 21, 2015 at 11:13:24AM +0800, Eddie Huang wrote: > > > > + /* set when doing the transfer */ > > > > + u16 irq_stat; /* interrupt status */ > > > > + unsigned int speed_hz; /* The speed in transfer */ > > > > + bool trans_stop; /* i2c transfer stop */ > > > > + enum mtk_trans_op op; > > > > + u16 msg_len; > > > > + u8 *msg_buf; /* pointer to msg data */ > > > > + u16 msg_aux_len; /* WRRD mode to set AUX_LEN register*/ > > > > + u16 addr; /* 7bit slave address, without read/write bit */ > > > Wouldn't it be easier to maintain a pointer to the message to be > > > transferred? > > I think use mtk_i2c pointer is more flexible than maintain a pointer to > > message. > Not sure you understood what I intended to suggest. My idea was to drop > > u16 msg_len; > u8 *msg_buf; > u16 msg_aux_len; // maybe > u16 addr; > > from struct mtk_i2c and add a struct i2c_msg *msg instead. Up to you to > decide. > Because this driver pass mtk_i2c pointer between functions, it's flexible to use any member of struct mtk_i2c. The good thing is avoid passing one more struct i2c_msg parameter between functions, or another struct copy. The bad thing is make struct mtk_i2c larger. I prefer to keep as it is now. Best Regards Eddie -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html