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]

 



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/
Obviously, a stop in middle will not work. For any read or write request,
2 byte rab write has to go first. We anyway have to separate rab part
in read transactions so write should be okay to have separate rab write
message.

All the client driver of this needs to create messages in pairs where first
one will always be a rab write and second one either read or write.

I can add a check for this condition in master_xfer if needed.

Do you still see any issue with this?

Thanks
Ajay

--
nvpublic
--
 
> 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