Hi Peter, > -----Original Message----- > From: linux-i2c-ow...@vger.kernel.org <linux-i2c-ow...@vger.kernel.org> > On Behalf Of Peter Rosin > Sent: Tuesday, September 11, 2018 1:55 AM > To: Ajay Gupta <aj...@nvidia.com>; w...@the-dreams.de; > heikki.kroge...@linux.intel.com > Cc: linux-usb@vger.kernel.org; linux-...@vger.kernel.org > 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