On Tue, Nov 29, 2022 at 07:34:58PM +0800, Binbin Zhou wrote: > 在 2022/11/28 21:24, Andy Shevchenko 写道: > > On Mon, Nov 28, 2022 at 08:03:40PM +0800, Binbin Zhou wrote: > > > 在 2022/11/25 18:41, Andy Shevchenko 写道: > > > > On Fri, Nov 25, 2022 at 04:55:20PM +0800, Binbin Zhou wrote: ... > > > > > +static int ls2x_i2c_start(struct i2c_adapter *adap, struct i2c_msg *msgs) > > > > > +{ > > > > > + struct ls2x_i2c_dev *dev = i2c_get_adapdata(adap); > > > > > + unsigned char addr = i2c_8bit_addr_from_msg(msgs); > > > > > + > > > > > + reinit_completion(&dev->cmd_complete); > > > > > + addr |= (msgs->flags & I2C_M_RD) ? 1 : 0; > > > > Why is this needed? > > > In the ls2x I2C controller, the bit 0 of TXR indicates the read/write status > > > when transferring the address. > > Yes, I understand this. I don't understand why do you need this twice. > > Are you saying that the "is_read" variable in ls2x_i2c_xfer_one() already > indicates the read/write state of data transfer? > > I just didn't think it was necessary to take "is_read" as an argument to > ls2x_i2c_start() at the time, since we could get it from "msg". Have you checked what i2c_8bit_addr_from_msg() is doing? -- With Best Regards, Andy Shevchenko