RE: [PATCH v11 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,

> -----Original Message-----
> From: linux-i2c-owner@xxxxxxxxxxxxxxx <linux-i2c-owner@xxxxxxxxxxxxxxx>
> On Behalf Of Peter Rosin
> Sent: Tuesday, September 11, 2018 1:55 AM
> To: Ajay Gupta <ajayg@xxxxxxxxxx>; wsa@xxxxxxxxxxxxx;
> heikki.krogerus@xxxxxxxxxxxxxxx
> Cc: linux-usb@xxxxxxxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v11 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
> 
> [I seem to have lost my local copy of the mail I'm responding to, so I copied
> bits of it from an archive and broke threading in the process, sorry about that]
That’s why was wondering how you got "goto stop" in "if (msgs[i].flags & I2C_M_RD) {"
Block at [1]. There is actually no stop when gpu_i2c_read() fails in master_xfer()

[1] https://marc.info/?l=linux-usb&m=153662481422174&w=2 
> 
> On 2018-09-11 00:22, Ajay Gupta wrote:
> >> Hmm, that goto stop is however not perfect. Ideally, you shouldn't
> >> issue stop if i == 0 and gpu_i2c_read failed
See above. There is no "stop"


Thanks
Ajay

--nvpublic
> > How can there be a read without rab write first?
> > AFAIU, gpu_i2c_read() can get called only with i=1 onward.
> 
> Well, you said that there could be other I2C devices on this I2C bus. I thought
> that meant that there could be other I2C client drivers using this I2C adapter.
> If so, then those drivers can issue I2C transfers where the first message is a
> read.
> 
> >> > +static int gpu_populate_client(struct gpu_i2c_dev *i2cd, int irq) {
> >> > +	static struct i2c_board_info gpu_ccgx_ucsi = {
> >> > +		I2C_BOARD_INFO("ccgx-ucsi", 0x8),
> >> > +	};
> >>
> >> Shouldn't this struct be dynamically allocated and attached to struct
> >> gpu_i2c_dev so that you (maybe) could have more than one instance?
> > Currently the structure is local variable. Will that not work in multi
> > instance cases?
> 
> Arrggh. NO!
> 
> A static variable in function scope behaves very much the same as a static
> variable in file scope. The only difference is that the visibility is different. All
> calls to the function gets the same gpu_ccgx_ucsi instance which means that
> instances will clobber each other.
> 
> You need to add a struct i2c_board_info (or a pointer to one) to struct
> gpu_i2c_dev so that it can be dynamically allocated.
> 
> 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