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
