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
