On 2018-09-10 19:28, Peter Rosin wrote: > On 2018-09-10 18:08, Ajay Gupta wrote: >> Hi Peter, >> >>>>> + >>>>> + if (msgs[i].flags & I2C_M_RD) { >>>>> + /* gpu_i2c_read has implicit start and stop */ >>>>> + status = gpu_i2c_read(i2cd, msgs[i].buf, msgs[i].len); >>>>> + if (status < 0) >>>>> + return status; >>>>> + } else { >>>>> + /* start on first write message */ >>>>> + if (i == 0) { >>>> >>>> This "if (i == 0)" test is completely bogus to me. I fail to see why >>>> the meat of the block should not happen for both writes in a >>>> double-write transfer. >>>> >>>> If the second message is a write, you do not issue any start nor do >>>> you write out the address for the second message. You want to generate >>>> the following for a transfer consisting of 2 write-messages: >>>> >>>> S Addr Wr [A] Data [A] ... S Addr Wr [A] Data [A] ... P >>>> ============= >>>> >>>> (where "..." denotes further optional "Data [A]" instances) >>>> >>>> As is, the stuff underlined by equal signs are not generated, at least >>>> as I read the code. >>>> >>>> This is what I meant in my comment around this area for the v9 patch. >>> >>> Oh, I just realized, this probably means that the ccg_write function in patch >>> 2/2 asks for the wrong thing. If this code actually works, the client driver >>> should probably ask for a single-message transfer consisting of the 2-byte rab >>> concatenated with the data buffer. >>> And that actually makes sense, there is no >>> reason to split the two (dependent) parts of the write into separate messages. >> That would require to create new buffer and copy data for each write request >> from UCSI core driver for sending UCSI command. This doesn't look proper way >> of doing it. > > Well, that's the way master_xfer works, so you will just have to live with it > if you do not want a stop in the middle. Bzzt, s/stop/restart/ Cheers, Peter