All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Rosin <peda@axentia.se>
To: Ajay Gupta <ajayg@nvidia.com>,
	wsa@the-dreams.de, heikki.krogerus@linux.intel.com
Cc: linux-usb@vger.kernel.org, linux-i2c@vger.kernel.org
Subject: [v11,1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Date: Tue, 11 Sep 2018 10:55:24 +0200	[thread overview]
Message-ID: <2a6fb720-777e-83a7-b5a0-73746e06011e@axentia.se> (raw)

[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

             reply	other threads:[~2018-09-11  8:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-11  8:55 Peter Rosin [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-09-12 10:19 [v11,1/2] i2c: buses: add i2c bus driver for NVIDIA GPU Peter Rosin
2018-09-11 16:53 Ajay Gupta
2018-09-11  4:16 Ajay Gupta
2018-09-11  0:13 Peter Rosin
2018-09-10 22:22 Ajay Gupta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2a6fb720-777e-83a7-b5a0-73746e06011e@axentia.se \
    --to=peda@axentia.se \
    --cc=ajayg@nvidia.com \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.