Re: [PATCH v10 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux