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.
Thanks
Ajay
--
nvpublic
--
>
> Otherwise, fixing the bug here will break your client driver, but two bugs that
> just happen to cancel each other out are not what we want...
>
> All assuming my hunch is correct, of course.
>
> Cheers,
> Peter
