[PATCH] i2c/i2c-dev: use dynamic minor allocation

[PATCH] i2c/i2c-dev: use dynamic minor allocation

[Sebastian Andrzej Siewior]

Right now i2c adapter 5 becomes minor 5 allocated. This is fine as long
as no adapter becomes a number > 256 allocated.
The Sodavile PCI driver uses (devfn << 3 | pci_bar) to come up with an
unique adapter number. So the first i2c adapter has the number 720.
This patch introduces dynamic minor allocation so the minor first
registered i2c adapter will be zero, next one one and so on. The name
which is exported to userland remains the same i.e. /dev/i2c-10 for
adapter number 10 (but its minor number may be zero and not 10).
I don't consider this as an ABI change however if someone does I could
add make this a CONFIG_ option and migrate in a year or so. i2c-tools
for instance abort on adapter number > 255 so we have to change this
in order to get the big numbers working.
Since the minors are not allocated dynamically I lowered the number of
max i2c devices to 32 so I don't waste much memory for the allocation.
If one has more than 32 devices then we could increase this again :)

Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Signed-off-by: Dirk Brandewie <dirk.brandewie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/i2c-dev.c |   54 ++++++++++++++++++++++++++++++++++++------------
 1 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index cec0f3b..57eda78 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -52,9 +52,10 @@ struct i2c_dev {
 	struct list_head list;
 	struct i2c_adapter *adap;
 	struct device *dev;
+	int minor;
 };
 
-#define I2C_MINORS	256
+#define I2C_MINORS	32
 static LIST_HEAD(i2c_dev_list);
 static DEFINE_SPINLOCK(i2c_dev_list_lock);
 
@@ -64,7 +65,7 @@ static struct i2c_dev *i2c_dev_get_by_minor(unsigned index)
 
 	spin_lock(&i2c_dev_list_lock);
 	list_for_each_entry(i2c_dev, &i2c_dev_list, list) {
-		if (i2c_dev->adap->nr == index)
+		if (i2c_dev->minor == index)
 			goto found;
 	}
 	i2c_dev = NULL;
@@ -73,22 +74,46 @@ found:
 	return i2c_dev;
 }
 
-static struct i2c_dev *get_free_i2c_dev(struct i2c_adapter *adap)
+static struct i2c_dev *i2c_dev_get_by_adapter(struct i2c_adapter *adap)
 {
 	struct i2c_dev *i2c_dev;
 
-	if (adap->nr >= I2C_MINORS) {
-		printk(KERN_ERR "i2c-dev: Out of device minors (%d)\n",
-		       adap->nr);
-		return ERR_PTR(-ENODEV);
+	spin_lock(&i2c_dev_list_lock);
+	list_for_each_entry(i2c_dev, &i2c_dev_list, list) {
+		if (i2c_dev->adap == adap)
+			goto found;
 	}
+	i2c_dev = NULL;
+found:
+	spin_unlock(&i2c_dev_list_lock);
+	return i2c_dev;
+}
+
+static DECLARE_BITMAP(minors, I2C_MINORS);
+
+static struct i2c_dev *get_free_i2c_dev(struct i2c_adapter *adap)
+{
+	struct i2c_dev *i2c_dev;
+	int minor;
+	int ret = 0;
 
 	i2c_dev = kzalloc(sizeof(*i2c_dev), GFP_KERNEL);
 	if (!i2c_dev)
-		return ERR_PTR(-ENOMEM);
-	i2c_dev->adap = adap;
+		return ERR_PTR(ret);
 
 	spin_lock(&i2c_dev_list_lock);
+	minor = find_first_zero_bit(minors, I2C_MINORS);
+	if (minor >= I2C_MINORS) {
+		spin_unlock(&i2c_dev_list_lock);
+		kfree(i2c_dev);
+		printk(KERN_ERR "i2c-dev: Out of device minors.\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	i2c_dev->adap = adap;
+	i2c_dev->minor = minor;
+	set_bit(minor, minors);
+
 	list_add_tail(&i2c_dev->list, &i2c_dev_list);
 	spin_unlock(&i2c_dev_list_lock);
 	return i2c_dev;
@@ -97,6 +122,7 @@ static struct i2c_dev *get_free_i2c_dev(struct i2c_adapter *adap)
 static void return_i2c_dev(struct i2c_dev *i2c_dev)
 {
 	spin_lock(&i2c_dev_list_lock);
+	clear_bit(i2c_dev->minor, minors);
 	list_del(&i2c_dev->list);
 	spin_unlock(&i2c_dev_list_lock);
 	kfree(i2c_dev);
@@ -541,7 +567,7 @@ static int i2cdev_attach_adapter(struct i2c_adapter *adap)
 
 	/* register this i2c device with the driver core */
 	i2c_dev->dev = device_create(i2c_dev_class, &adap->dev,
-				     MKDEV(I2C_MAJOR, adap->nr), NULL,
+				     MKDEV(I2C_MAJOR, i2c_dev->minor), NULL,
 				     "i2c-%d", adap->nr);
 	if (IS_ERR(i2c_dev->dev)) {
 		res = PTR_ERR(i2c_dev->dev);
@@ -552,10 +578,10 @@ static int i2cdev_attach_adapter(struct i2c_adapter *adap)
 		goto error_destroy;
 
 	pr_debug("i2c-dev: adapter [%s] registered as minor %d\n",
-		 adap->name, adap->nr);
+		 adap->name, i2c_dev->minor);
 	return 0;
 error_destroy:
-	device_destroy(i2c_dev_class, MKDEV(I2C_MAJOR, adap->nr));
+	device_destroy(i2c_dev_class, MKDEV(I2C_MAJOR, i2c_dev->minor));
 error:
 	return_i2c_dev(i2c_dev);
 	return res;
@@ -565,13 +591,13 @@ static int i2cdev_detach_adapter(struct i2c_adapter *adap)
 {
 	struct i2c_dev *i2c_dev;
 
-	i2c_dev = i2c_dev_get_by_minor(adap->nr);
+	i2c_dev = i2c_dev_get_by_adapter(adap);
 	if (!i2c_dev) /* attach_adapter must have failed */
 		return 0;
 
 	device_remove_file(i2c_dev->dev, &dev_attr_name);
 	return_i2c_dev(i2c_dev);
-	device_destroy(i2c_dev_class, MKDEV(I2C_MAJOR, adap->nr));
+	device_destroy(i2c_dev_class, MKDEV(I2C_MAJOR, i2c_dev->minor));
 
 	pr_debug("i2c-dev: adapter [%s] unregistered\n", adap->name);
 	return 0;
-- 
1.7.3.2

Re: [PATCH] i2c/i2c-dev: use dynamic minor allocation

[Jean Delvare]

Hi Sebastian,

On Wed, 24 Nov 2010 22:23:08 +0100, Sebastian Andrzej Siewior wrote:
> Right now i2c adapter 5 becomes minor 5 allocated. This is fine as long
> as no adapter becomes a number > 256 allocated.

Why would it be a problem to have a minor number > 256 (or more likely
you meant > 255)? Minors beyond 8 bit are supported since quite some
time, aren't they?

> The Sodavile PCI driver uses (devfn << 3 | pci_bar) to come up with an
> unique adapter number. So the first i2c adapter has the number 720.

If you know that the first adapter has the number 720, then you could
simply use (devfn << 3 | pci_bar) - 720 as the adapter number. I don't
know how many such PCI devices a system can have, but presumably the
256 minor limit would be sufficient.

But my main question is: why do you want a unique (or you probably
meant predictable - adapter numbers are already unique by design)
adapter number in the first place? Other systems apparently are doing
just fine without this.

Also, what if another i2c adapter driver comes up with its own idea of how
adapters should be numbered, and its numbering scheme collides with
your driver?

Fixed i2c adapter numbers are already supported, but it's up to the
platform initialization code to define them, not the i2c adapter driver.

> This patch introduces dynamic minor allocation so the minor first
> registered i2c adapter will be zero, next one one and so on. The name
> which is exported to userland remains the same i.e. /dev/i2c-10 for
> adapter number 10 (but its minor number may be zero and not 10).

Are there other subsystems doing this already?

> I don't consider this as an ABI change however if someone does I could
> add make this a CONFIG_ option and migrate in a year or so. i2c-tools
> for instance abort on adapter number > 255 so we have to change this
> in order to get the big numbers working.

i2c-tools could be updated if needed. As for a CONFIG option,
CONFIG_I2C_COMPAT could be used for this. But at this point I still
need to be convinced that there's a need at all.

> Since the minors are not allocated dynamically I lowered the number of
> max i2c devices to 32 so I don't waste much memory for the allocation.
> If one has more than 32 devices then we could increase this again :)

Systems with more than 32 I2C adapters already exist, so lowering to 32
is not acceptable.

-- 
Jean Delvare

Re: [PATCH] i2c/i2c-dev: use dynamic minor allocation

[Sebastian Andrzej Siewior]

Jean Delvare wrote:
> Hi Sebastian,
Hi Jean,

> On Wed, 24 Nov 2010 22:23:08 +0100, Sebastian Andrzej Siewior wrote:
>> Right now i2c adapter 5 becomes minor 5 allocated. This is fine as long
>> as no adapter becomes a number > 256 allocated.
> 
> Why would it be a problem to have a minor number > 256 (or more likely
> you meant > 255)? Minors beyond 8 bit are supported since quite some
> time, aren't they?

Argh. Now that you mention it. I knew it was 8bit and after looking it up
MINOR() was still 8bit. This was the userland export I was loking at. So
in-kernel MINOR() has 20bits. Replacing I2C_MINORS with MINORMASK should
do the job.

>> The Sodavile PCI driver uses (devfn << 3 | pci_bar) to come up with an
>> unique adapter number. So the first i2c adapter has the number 720.
> 
> If you know that the first adapter has the number 720, then you could
> simply use (devfn << 3 | pci_bar) - 720 as the adapter number. I don't
> know how many such PCI devices a system can have, but presumably the
> 256 minor limit would be sufficient.
Yup. But this device could be included on a different SoC with a different
PCI bus number and this hack could end up with a value <0.

> But my main question is: why do you want a unique (or you probably
> meant predictable - adapter numbers are already unique by design)
> adapter number in the first place? Other systems apparently are doing
> just fine without this.
Both. I use this number for the device id. This one has to remain unique
or sysfs goes crazy. So using the PCI number looked like a good idea.
The PXA I2C driver uses this number for the adapt number.
I needed this to be predictable in order to match match my board
description with the correct i2c controller. This is however no longer the
case because I use the device tree for this kind of information.
So other issue would be i2c-dev where userland wants always the same
device.

> Also, what if another i2c adapter driver comes up with its own idea of how
> adapters should be numbered, and its numbering scheme collides with
> your driver?
Too bad. I though that I will be on the safe side using using the PCI
slot+device number. This is not a problem for the device id just the
adapter number. For that I could add a flag to platform data to use
i2c_add_adapter instead of i2c_add_numbered_adapter. Not sure what I could
break if I change it unconditionally. Userland could still look up
sys/class/i2c-dev/i2c-X to find out which device it is talking to.
Any comments?

> Fixed i2c adapter numbers are already supported, but it's up to the
> platform initialization code to define them, not the i2c adapter driver.
I don't want platform init code.

>> This patch introduces dynamic minor allocation so the minor first
>> registered i2c adapter will be zero, next one one and so on. The name
>> which is exported to userland remains the same i.e. /dev/i2c-10 for
>> adapter number 10 (but its minor number may be zero and not 10).
> 
> Are there other subsystems doing this already?
Well, spi does it for instance. The userland interface gets spidevX.Y
where X is the SPI bus number and Y is the chip select number. The minor
for those is dynamically allocated.

> Systems with more than 32 I2C adapters already exist, so lowering to 32
> is not acceptable.

Ah, I haven't seen that comming.

Sebastian

Re: [PATCH] i2c/i2c-dev: use dynamic minor allocation

[Jean Delvare]

Hi Sebastian,

On Thu, 25 Nov 2010 11:43:54 +0100, Sebastian Andrzej Siewior wrote:
> Jean Delvare wrote:
> > On Wed, 24 Nov 2010 22:23:08 +0100, Sebastian Andrzej Siewior wrote:
> >> Right now i2c adapter 5 becomes minor 5 allocated. This is fine as long
> >> as no adapter becomes a number > 256 allocated.
> > 
> > Why would it be a problem to have a minor number > 256 (or more likely
> > you meant > 255)? Minors beyond 8 bit are supported since quite some
> > time, aren't they?
> 
> Argh. Now that you mention it. I knew it was 8bit and after looking it up
> MINOR() was still 8bit. This was the userland export I was loking at. So
> in-kernel MINOR() has 20bits. Replacing I2C_MINORS with MINORMASK should
> do the job.

... but would require updating i2c-tools. That being said, we probably
want to do that anyway, as this limit is pretty arbitrary and doesn't
add any value.

> >> The Sodavile PCI driver uses (devfn << 3 | pci_bar) to come up with an
> >> unique adapter number. So the first i2c adapter has the number 720.
> > 
> > If you know that the first adapter has the number 720, then you could
> > simply use (devfn << 3 | pci_bar) - 720 as the adapter number. I don't
> > know how many such PCI devices a system can have, but presumably the
> > 256 minor limit would be sufficient.
>
> Yup. But this device could be included on a different SoC with a different
> PCI bus number and this hack could end up with a value <0.

Good point.

> > But my main question is: why do you want a unique (or you probably
> > meant predictable - adapter numbers are already unique by design)
> > adapter number in the first place? Other systems apparently are doing
> > just fine without this.
>
> Both. I use this number for the device id. This one has to remain unique
> or sysfs goes crazy.

Which "device id" are you talking about, please? Of course sysfs
doesn't accept duplicate entries, but as I said already, uniqueness is
already guaranteed by i2c-dev. Your problem is predictability.

> So using the PCI number looked like a good idea.

It's one idea amongst others, and it doesn't strike me as particularly
good, because it works only by accident: you lost uniqueness while
adding predictability.

> The PXA I2C driver uses this number for the adapt number.

I got that, thanks. Otherwise you wouldn't be worried about i2c-dev.

> I needed this to be predictable in order to match match my board
> description with the correct i2c controller.

Precisions, please. "Match my board description" is awfully vague.
Don't hesitate to be technical, I'm sure I can understand you ;)

> This is however no longer the
> case because I use the device tree for this kind of information.

"This kind of information" is again too vague to be helpful.

> So other issue would be i2c-dev where userland wants always the same
> device.

This shouldn't be an issue. i2c tools support passing adapters by name,
so all your driver must do is make sure every i2c adapter will have its
own, specific name. The traditional method is to include the base
address, but PCI devfn and BAR would work too.

> > Also, what if another i2c adapter driver comes up with its own idea of how
> > adapters should be numbered, and its numbering scheme collides with
> > your driver?
>
> Too bad. I though that I will be on the safe side using using the PCI
> slot+device number.

This can't be safe until all devices in the world are PCI devices and
all i2c drivers agree to stick to this rule (i.e. it is enforced by
i2c-core.) This isn't going to happen anytime soon, I'm afraid.

> This is not a problem for the device id just the adapter number.

I have no idea what you mean. Please explain.

> For that I could add a flag to platform data to use
> i2c_add_adapter instead of i2c_add_numbered_adapter. Not sure what I could

So you have platform data? Where does it come from?

> break if I change it unconditionally. Userland could still look up
> sys/class/i2c-dev/i2c-X to find out which device it is talking to.
> Any comments?

I can't comment on something I don't understand, sorry. Please describe
actual use cases in detail, otherwise I can't help you.

> > Fixed i2c adapter numbers are already supported, but it's up to the
> > platform initialization code to define them, not the i2c adapter driver.
>
> I don't want platform init code.

Why? This is what (almost) everybody does when predictable i2c adapter
numbers are needed. And you said you have platform data, so presumably
you already have some form of platform init code (even if it's generic
code with a custom device tree.)

The alternative is to call i2c_new_device() in the i2c bus driver
directly, but this could become messy quickly if more people start
doing it.

> >> This patch introduces dynamic minor allocation so the minor first
> >> registered i2c adapter will be zero, next one one and so on. The name
> >> which is exported to userland remains the same i.e. /dev/i2c-10 for
> >> adapter number 10 (but its minor number may be zero and not 10).
> > 
> > Are there other subsystems doing this already?
>
> Well, spi does it for instance. The userland interface gets spidevX.Y
> where X is the SPI bus number and Y is the chip select number. The minor
> for those is dynamically allocated.

I didn't know SPI was using per-device dev nodes. Of course static
nodes don't work well in that case.

> > Systems with more than 32 I2C adapters already exist, so lowering to 32
> > is not acceptable.
> 
> Ah, I haven't seen that comming.

In all honesty, I'm not sure if I really saw such a system yet. I seem
to remember so but I can't find a trace again. But seeing this:

http://www.spinics.net/lists/lm-sensors/msg30206.html

it would only take 2 Intel graphics chips and a TV adapter to reach 34
i2c adapters. And with multiplexing support now available, consuming i2c
adapter numbers will be easier than ever. So setting 32 as a limit is
no way to go.

-- 
Jean Delvare

Re: [sodaville] [PATCH] i2c/i2c-dev: use dynamic minor allocation

[Sebastian Andrzej Siewior]

* Jean Delvare | 2010-11-25 22:52:46 [+0100]:

>Hi Sebastian,
Hi Jean,

>> > But my main question is: why do you want a unique (or you probably
>> > meant predictable - adapter numbers are already unique by design)
>> > adapter number in the first place? Other systems apparently are doing
>> > just fine without this.
>>
>> Both. I use this number for the device id. This one has to remain unique
>> or sysfs goes crazy.
>
>Which "device id" are you talking about, please? Of course sysfs
The pci driver [0] has in add_i2c_device() the following construct:

     pdev->id = dev->devfn << 3 | bar;

I'm talking about this device id. Here I have to remain unique and here I
though using the pci id would be a good idea. There should be no other
device using "pxa2xx-i2c" as the device name so I should be safe here.

>doesn't accept duplicate entries, but as I said already, uniqueness is

>Precisions, please. "Match my board description" is awfully vague.
>Don't hesitate to be technical, I'm sure I can understand you ;)
Sorry didn't meant to. As you probably know, by "board description" I
meant struct i2c_board_info & i2c_register_board_info(). This is now
covered by of_i2c_register_devices(&i2c->adap)

>> > Also, what if another i2c adapter driver comes up with its own idea of how
>> > adapters should be numbered, and its numbering scheme collides with
>> > your driver?
>>
>> Too bad. I though that I will be on the safe side using using the PCI
>> slot+device number.
>
>This can't be safe until all devices in the world are PCI devices and
>all i2c drivers agree to stick to this rule (i.e. it is enforced by
>i2c-core.) This isn't going to happen anytime soon, I'm afraid.
Now I see what you mean.

>> > Fixed i2c adapter numbers are already supported, but it's up to the
>> > platform initialization code to define them, not the i2c adapter driver.
>>
>> I don't want platform init code.
>
>Why? This is what (almost) everybody does when predictable i2c adapter
>numbers are needed. And you said you have platform data, so presumably
>you already have some form of platform init code (even if it's generic
>code with a custom device tree.)

This sounds good. So I get "pdev->id" from the device tree which should
solve my trouble I have so far.
Thank you.

[0] http://article.gmane.org/gmane.linux.drivers.i2c/7260

Sebastian

Re: [sodaville] [PATCH] i2c/i2c-dev: use dynamic minor allocation

[Jean Delvare]

Hi Sebastian,

On Fri, 26 Nov 2010 19:03:25 +0100, Sebastian Andrzej Siewior wrote:
> * Jean Delvare | 2010-11-25 22:52:46 [+0100]:
> 
> >Hi Sebastian,
> Hi Jean,
> 
> >> > But my main question is: why do you want a unique (or you probably
> >> > meant predictable - adapter numbers are already unique by design)
> >> > adapter number in the first place? Other systems apparently are doing
> >> > just fine without this.
> >>
> >> Both. I use this number for the device id. This one has to remain unique
> >> or sysfs goes crazy.
> >
> >Which "device id" are you talking about, please? Of course sysfs
> The pci driver [0] has in add_i2c_device() the following construct:
> 
>      pdev->id = dev->devfn << 3 | bar;
> 
> I'm talking about this device id. Here I have to remain unique and here I

Huh? Is pdev a platform device? So you are instantiating platform
devices off a PCI device, and in turn each platform device gets to
create an i2c_adapter device?

Now I understand why you needed to craft a unique id. I don't quite get
why you came up with such a complicated setup in the first place
though. What's wrong with just making the i2c_adapter devices direct
children of the PCI device? Is this a limitation of
of_i2c_register_devices() that it needs platform devices to operate on?

> though using the pci id would be a good idea. There should be no other
> device using "pxa2xx-i2c" as the device name so I should be safe here.

Correct.

> >doesn't accept duplicate entries, but as I said already, uniqueness is
> >
> >Precisions, please. "Match my board description" is awfully vague.
> >Don't hesitate to be technical, I'm sure I can understand you ;)
> Sorry didn't meant to. As you probably know, by "board description" I
> meant struct i2c_board_info & i2c_register_board_info(). This is now
> covered by of_i2c_register_devices(&i2c->adap)

OK, it's clearer now, thanks.

> >> > Fixed i2c adapter numbers are already supported, but it's up to the
> >> > platform initialization code to define them, not the i2c adapter driver.
> >>
> >> I don't want platform init code.
> >
> >Why? This is what (almost) everybody does when predictable i2c adapter
> >numbers are needed. And you said you have platform data, so presumably
> >you already have some form of platform init code (even if it's generic
> >code with a custom device tree.)
> 
> This sounds good. So I get "pdev->id" from the device tree which should
> solve my trouble I have so far.

Sounds good. And now you no longer care about i2c adapter numbers -
they don't have to be correlated to your platform device numbers.

Meanwhile I've updated i2c-tools to support dev minors beyond 255. You
should no longer need this, but at least it's ready if we ever need it.
Who knows, maybe we'll see a system with more than 255 I2C bus segments
someday.

-- 
Jean Delvare

Re: [sodaville] [PATCH] i2c/i2c-dev: use dynamic minor allocation

[Sebastian Andrzej Siewior]

* Jean Delvare | 2010-11-26 20:46:32 [+0100]:

>Hi Sebastian,
Hi Jean,

>Huh? Is pdev a platform device? So you are instantiating platform
>devices off a PCI device, and in turn each platform device gets to
>create an i2c_adapter device?
Yup, exactly.

>Now I understand why you needed to craft a unique id. I don't quite get
>why you came up with such a complicated setup in the first place
>though. What's wrong with just making the i2c_adapter devices direct
>children of the PCI device? Is this a limitation of
>of_i2c_register_devices() that it needs platform devices to operate on?
No. In the end we have one driver which may get probed viaa  PCI device
or a platform device. This leaves me two choices: 
- create a platform device out of the PCI device like I did
- add PCI probing code to the platform driver so it provides two
  interfaces

If you are look at the i2c_pxa_probe() function then you'll notice
that there is no real cut between obtaining ressource informations and
setting up the i2c device. So this would require to split this function
into pieces and share the code between those two interfaces. While
looking at it, I decided against the split.
This way the patch way smaller and simpler. I don't see a disadvantage
with the platform device here. Well maybe the pdev->id problem :)

>Meanwhile I've updated i2c-tools to support dev minors beyond 255. You
>should no longer need this, but at least it's ready if we ever need it.
>Who knows, maybe we'll see a system with more than 255 I2C bus segments
>someday.

Yeah, I don't right now.

Sebastian

Re: [sodaville] [PATCH] i2c/i2c-dev: use dynamic minor allocation

[Ben Dooks]

On Fri, Nov 26, 2010 at 08:46:32PM +0100, Jean Delvare wrote:
> Hi Sebastian,
> 
> On Fri, 26 Nov 2010 19:03:25 +0100, Sebastian Andrzej Siewior wrote:
> > * Jean Delvare | 2010-11-25 22:52:46 [+0100]:
> > 
> > >Hi Sebastian,
> > Hi Jean,
> > 
> > >> > But my main question is: why do you want a unique (or you probably
> > >> > meant predictable - adapter numbers are already unique by design)
> > >> > adapter number in the first place? Other systems apparently are doing
> > >> > just fine without this.
> > >>
> > >> Both. I use this number for the device id. This one has to remain unique
> > >> or sysfs goes crazy.
> > >
> > >Which "device id" are you talking about, please? Of course sysfs
> > The pci driver [0] has in add_i2c_device() the following construct:
> > 
> >      pdev->id = dev->devfn << 3 | bar;
> > 
> > I'm talking about this device id. Here I have to remain unique and here I
> 
> Huh? Is pdev a platform device? So you are instantiating platform
> devices off a PCI device, and in turn each platform device gets to
> create an i2c_adapter device?

I think this is due to re-instantiating a platform_device based driver
without adding any extra driver registration to it.

I'm considering the idea that the relevant driver registeration should
be put into the i2c-pxa driver and select the relevant things...
 
> Now I understand why you needed to craft a unique id. I don't quite get
> why you came up with such a complicated setup in the first place
> though. What's wrong with just making the i2c_adapter devices direct
> children of the PCI device? Is this a limitation of
> of_i2c_register_devices() that it needs platform devices to operate on?
> 
> > though using the pci id would be a good idea. There should be no other
> > device using "pxa2xx-i2c" as the device name so I should be safe here.

Or add a new platform device name, it isn't difficult.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.