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]

 



[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]

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 
> 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 Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux