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.
Cheers,
Peter
