[PATCH] i2c-eg20t: use dynamically registered adapter number

[PATCH] i2c-eg20t: use dynamically registered adapter number

[Yadi Hu]

From: Hu Yadi <Yadi.hu@windriver.com>

The eg20t driver uses i2c_add_numbered_adapter() to register adapter:

    pch_adap->nr = i;
    ret = i2c_add_numbered_adapter(pch_adap);

Variable i is assigned to 0, it means that i2c_eg20t is the first adapter
by default. if another adapter registers before eg20t, above code return
error for index conflict:

    i2c_eg20t 0000:05:0c.2: pch_i2c_probe :i2c_add_adapter[ch:0] FAILED
    i2c_eg20t: probe of 0000:05:0c.2 failed with error -16

So, we can replace i2c_add_numbered_adapter() with i2c_add_adapter()
interface.since it dynamically allocates the index number.

Signed-off-by: Hu Yadi <Yadi.hu@windriver.com>
---
 drivers/i2c/busses/i2c-eg20t.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c
index 7a51ddc..2f4c2af 100644
--- a/drivers/i2c/busses/i2c-eg20t.c
+++ b/drivers/i2c/busses/i2c-eg20t.c
@@ -913,8 +913,7 @@ static int __devinit pch_i2c_probe(struct pci_dev *pdev,
 
 		pch_i2c_init(&adap_info->pch_data[i]);
 
-		pch_adap->nr = i;
-		ret = i2c_add_numbered_adapter(pch_adap);
+               ret = i2c_add_adapter(pch_adap);
 		if (ret) {
 			pch_pci_err(pdev, "i2c_add_adapter[ch:%d] FAILED\n", i);
 			goto err_add_adapter;
-- 
1.7.5.4

Re: [PATCH] i2c-eg20t: use dynamically registered adapter number

[Jean Delvare]

Hi Yadi,

On Tue, 23 Aug 2016 17:05:58 +0800, Yadi Hu wrote:
> From: Hu Yadi <Yadi.hu@windriver.com>
> 
> The eg20t driver uses i2c_add_numbered_adapter() to register adapter:
> 
>     pch_adap->nr = i;
>     ret = i2c_add_numbered_adapter(pch_adap);
> 
> Variable i is assigned to 0, it means that i2c_eg20t is the first adapter
> by default. if another adapter registers before eg20t, above code return
> error for index conflict:
> 
>     i2c_eg20t 0000:05:0c.2: pch_i2c_probe :i2c_add_adapter[ch:0] FAILED
>     i2c_eg20t: probe of 0000:05:0c.2 failed with error -16
> 
> So, we can replace i2c_add_numbered_adapter() with i2c_add_adapter()
> interface.since it dynamically allocates the index number.

This does the exact opposite of:

commit 07e8a51ff68353e01d795cceafbac9f54c49132b
Author: Feng Tang <feng.tang@intel.com>
Date:   Thu Jan 12 15:38:02 2012 +0800

    i2c-eg20t: use i2c_add_numbered_adapter to get a fixed bus number

So my understanding is that you are not supposed to register another
i2c bus before the eg20t buses. What is the conflicting driver? Is it
creating the buses with static number or not?

Looking at

commit 03bde7c31a360f814ca42101d60563b1b803dca1
Author: Wolfram Sang <wsa+renesas@sang-engineering.com>
Date:   Thu Mar 12 17:17:59 2015 +0100

    i2c: busses with dynamic ids should start after fixed ids for DT

it could be that you need to set some OF attribute to reserve i2c bus
numbers <= 1 for static usage. Assuming you use OF. Or is it automatic,
Wolfram?

If not, it may make sense to add a helper function exposing
__i2c_first_dynamic_bus_num to drivers (something like
i2c_is_dynamic_bus_num().) After all, i2c_add_numbered_adapter() mostly
makes sense if static i2c device definitions exist. If not,
i2c_add_adapter() is just as good. So something like:

	if (i2c_is_dynamic_bus_num(i))
		ret = i2c_add_adapter(pch_adap);
	else {
		pch_adap->nr = i;
		ret = i2c_add_numbered_adapter(pch_adap);
	}

may make sense. Unless someone has a better idea.

> 
> Signed-off-by: Hu Yadi <Yadi.hu@windriver.com>
> ---
>  drivers/i2c/busses/i2c-eg20t.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c
> index 7a51ddc..2f4c2af 100644
> --- a/drivers/i2c/busses/i2c-eg20t.c
> +++ b/drivers/i2c/busses/i2c-eg20t.c
> @@ -913,8 +913,7 @@ static int __devinit pch_i2c_probe(struct pci_dev *pdev,
>  
>  		pch_i2c_init(&adap_info->pch_data[i]);
>  
> -		pch_adap->nr = i;
> -		ret = i2c_add_numbered_adapter(pch_adap);
> +               ret = i2c_add_adapter(pch_adap);

Coding style is wrong here. Please use tabs, as ./scripts/checkpatch.pl
tells you. Always check your patches with this script before you post
them, thanks.

>  		if (ret) {
>  			pch_pci_err(pdev, "i2c_add_adapter[ch:%d] FAILED\n", i);
>  			goto err_add_adapter;


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c-eg20t: use dynamically registered adapter number

[Yadi]

On 2016年08月26日 23:30, Jean Delvare wrote:
> Hi Yadi,
>
> On Tue, 23 Aug 2016 17:05:58 +0800, Yadi Hu wrote:
>> From: Hu Yadi <Yadi.hu@windriver.com>
>>
>> The eg20t driver uses i2c_add_numbered_adapter() to register adapter:
>>
>>      pch_adap->nr = i;
>>      ret = i2c_add_numbered_adapter(pch_adap);
>>
>> Variable i is assigned to 0, it means that i2c_eg20t is the first adapter
>> by default. if another adapter registers before eg20t, above code return
>> error for index conflict:
>>
>>      i2c_eg20t 0000:05:0c.2: pch_i2c_probe :i2c_add_adapter[ch:0] FAILED
>>      i2c_eg20t: probe of 0000:05:0c.2 failed with error -16
>>
>> So, we can replace i2c_add_numbered_adapter() with i2c_add_adapter()
>> interface.since it dynamically allocates the index number.
> This does the exact opposite of:
>
> commit 07e8a51ff68353e01d795cceafbac9f54c49132b
> Author: Feng Tang <feng.tang@intel.com>
> Date:   Thu Jan 12 15:38:02 2012 +0800
>
>      i2c-eg20t: use i2c_add_numbered_adapter to get a fixed bus number
>
> So my understanding is that you are not supposed to register another
> i2c bus before the eg20t buses. What is the conflicting driver? Is it
> creating the buses with static number or not?
I am using one Kontron M2M, on which both i2c-eg20t and i2c-kempld are 
used simultaneously.
since kempld always is firstly initialized and configured to register 
dynamically, it always occupied the first bus-id. i.e. 0.
consequently, eg20t will complain to fail to register for no space for 
"0" i2c-bus.


>
> Looking at
>
> commit 03bde7c31a360f814ca42101d60563b1b803dca1
> Author: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Date:   Thu Mar 12 17:17:59 2015 +0100
>
>      i2c: busses with dynamic ids should start after fixed ids for DT
>
> it could be that you need to set some OF attribute to reserve i2c bus
> numbers <= 1 for static usage. Assuming you use OF. Or is it automatic,
> Wolfram?
>
> If not, it may make sense to add a helper function exposing
> __i2c_first_dynamic_bus_num to drivers (something like
> i2c_is_dynamic_bus_num().) After all, i2c_add_numbered_adapter() mostly
> makes sense if static i2c device definitions exist. If not,
> i2c_add_adapter() is just as good. So something like:
>
> 	if (i2c_is_dynamic_bus_num(i))
> 		ret = i2c_add_adapter(pch_adap);
> 	else {
> 		pch_adap->nr = i;
> 		ret = i2c_add_numbered_adapter(pch_adap);
> 	}
>
> may make sense. Unless someone has a better idea.

I totally agree with you and send off V2 patch soon per your suggestion.


Yadi
>
>> Signed-off-by: Hu Yadi <Yadi.hu@windriver.com>
>> ---
>>   drivers/i2c/busses/i2c-eg20t.c |    3 +--
>>   1 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c
>> index 7a51ddc..2f4c2af 100644
>> --- a/drivers/i2c/busses/i2c-eg20t.c
>> +++ b/drivers/i2c/busses/i2c-eg20t.c
>> @@ -913,8 +913,7 @@ static int __devinit pch_i2c_probe(struct pci_dev *pdev,
>>   
>>   		pch_i2c_init(&adap_info->pch_data[i]);
>>   
>> -		pch_adap->nr = i;
>> -		ret = i2c_add_numbered_adapter(pch_adap);
>> +               ret = i2c_add_adapter(pch_adap);
> Coding style is wrong here. Please use tabs, as ./scripts/checkpatch.pl
> tells you. Always check your patches with this script before you post
> them, thanks.
>
>>   		if (ret) {
>>   			pch_pci_err(pdev, "i2c_add_adapter[ch:%d] FAILED\n", i);
>>   			goto err_add_adapter;
>

Re: [PATCH] i2c-eg20t: use dynamically registered adapter number

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2412 bytes --]


> > The eg20t driver uses i2c_add_numbered_adapter() to register adapter:
> > 
> >     pch_adap->nr = i;
> >     ret = i2c_add_numbered_adapter(pch_adap);
> > 
> > Variable i is assigned to 0, it means that i2c_eg20t is the first adapter
> > by default. if another adapter registers before eg20t, above code return
> > error for index conflict:
> > 
> >     i2c_eg20t 0000:05:0c.2: pch_i2c_probe :i2c_add_adapter[ch:0] FAILED
> >     i2c_eg20t: probe of 0000:05:0c.2 failed with error -16
> > 
> > So, we can replace i2c_add_numbered_adapter() with i2c_add_adapter()
> > interface.since it dynamically allocates the index number.
> 
> This does the exact opposite of:
> 
> commit 07e8a51ff68353e01d795cceafbac9f54c49132b
> Author: Feng Tang <feng.tang@intel.com>
> Date:   Thu Jan 12 15:38:02 2012 +0800
> 
>     i2c-eg20t: use i2c_add_numbered_adapter to get a fixed bus number

We have the same problem with i2c-pasemi:
http://www.spinics.net/lists/linux-i2c/msg25761.html

PCI + i2c_add_numbered_adapter + another device adding i2c busses

And we cannot simply change back to i2c_add_adapter() because it will
regress on those systems which originally wanted to have
i2c_add_numbered_adapter().

> Looking at
> 
> commit 03bde7c31a360f814ca42101d60563b1b803dca1
> Author: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Date:   Thu Mar 12 17:17:59 2015 +0100
> 
>     i2c: busses with dynamic ids should start after fixed ids for DT
> 
> it could be that you need to set some OF attribute to reserve i2c bus
> numbers <= 1 for static usage. Assuming you use OF. Or is it automatic,
> Wolfram?

No, you need to define an alias in the devicetree. I don't think this
platform is a DT user, though?

> If not, it may make sense to add a helper function exposing
> __i2c_first_dynamic_bus_num to drivers (something like
> i2c_is_dynamic_bus_num().) After all, i2c_add_numbered_adapter() mostly
> makes sense if static i2c device definitions exist. If not,
> i2c_add_adapter() is just as good. So something like:
> 
> 	if (i2c_is_dynamic_bus_num(i))
> 		ret = i2c_add_adapter(pch_adap);
> 	else {
> 		pch_adap->nr = i;
> 		ret = i2c_add_numbered_adapter(pch_adap);
> 	}
> 
> may make sense. Unless someone has a better idea.

Interesting idea. Need to think about it some more. I didn't have a
proper solution so far...

Thanks,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c-eg20t: use dynamically registered adapter number

[Yadi]

On 2016年09月18日 05:49, Wolfram Sang wrote:
>>> The eg20t driver uses i2c_add_numbered_adapter() to register adapter:
>>>
>>>      pch_adap->nr = i;
>>>      ret = i2c_add_numbered_adapter(pch_adap);
>>>
>>> Variable i is assigned to 0, it means that i2c_eg20t is the first adapter
>>> by default. if another adapter registers before eg20t, above code return
>>> error for index conflict:
>>>
>>>      i2c_eg20t 0000:05:0c.2: pch_i2c_probe :i2c_add_adapter[ch:0] FAILED
>>>      i2c_eg20t: probe of 0000:05:0c.2 failed with error -16
>>>
>>> So, we can replace i2c_add_numbered_adapter() with i2c_add_adapter()
>>> interface.since it dynamically allocates the index number.
>> This does the exact opposite of:
>>
>> commit 07e8a51ff68353e01d795cceafbac9f54c49132b
>> Author: Feng Tang <feng.tang@intel.com>
>> Date:   Thu Jan 12 15:38:02 2012 +0800
>>
>>      i2c-eg20t: use i2c_add_numbered_adapter to get a fixed bus number
> We have the same problem with i2c-pasemi:
> http://www.spinics.net/lists/linux-i2c/msg25761.html
>
> PCI + i2c_add_numbered_adapter + another device adding i2c busses
>
> And we cannot simply change back to i2c_add_adapter() because it will
> regress on those systems which originally wanted to have
> i2c_add_numbered_adapter().

I think the is one totally different issue with  i2c-pasemi, in which  
PCI device prior to probed  occupied bus number supposed to be used by 
i2c-pasemi.

For my case, i2c-kempld register dynamically  itself by default, 
consequently it would occupied i2c bus 0 and cause to fails for adding 
new i2c bus with i2c_add_number_adpater(0).
although  i2c-kempld provide variable 'i2c_bus' to control bus 
assignment,  it isso inconvenient we have to manually modify it to avoid 
possible conflict. so the reasonable solution looks like:

i2c-kempld.c

i2c->adap.nr = i2c_bus;

         if (i2c_bus >= -1)  /* using pci id to register if default=-1 
for dynamic assignment*/
             i2c->adap.n = pdev->id;
         ret = i2c_add_numbered_adapter(&i2c->adap);


how do you like it?

Yadi


>
>> Looking at
>>
>> commit 03bde7c31a360f814ca42101d60563b1b803dca1
>> Author: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Date:   Thu Mar 12 17:17:59 2015 +0100
>>
>>      i2c: busses with dynamic ids should start after fixed ids for DT
>>
>> it could be that you need to set some OF attribute to reserve i2c bus
>> numbers <= 1 for static usage. Assuming you use OF. Or is it automatic,
>> Wolfram?
> No, you need to define an alias in the devicetree. I don't think this
> platform is a DT user, though?
>
>> If not, it may make sense to add a helper function exposing
>> __i2c_first_dynamic_bus_num to drivers (something like
>> i2c_is_dynamic_bus_num().) After all, i2c_add_numbered_adapter() mostly
>> makes sense if static i2c device definitions exist. If not,
>> i2c_add_adapter() is just as good. So something like:
>>
>> 	if (i2c_is_dynamic_bus_num(i))
>> 		ret = i2c_add_adapter(pch_adap);
>> 	else {
>> 		pch_adap->nr = i;
>> 		ret = i2c_add_numbered_adapter(pch_adap);
>> 	}
>>
>> may make sense. Unless someone has a better idea.
> Interesting idea. Need to think about it some more. I didn't have a
> proper solution so far...
>
> Thanks,
>
>     Wolfram
>

Re: [PATCH] i2c-eg20t: use dynamically registered adapter number

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]


> For my case, i2c-kempld register dynamically  itself by default,
> consequently it would occupied i2c bus 0 and cause to fails for adding new
> i2c bus with i2c_add_number_adpater(0).

This is exactly the PASEMI case as well. A video card got probed before
the pasemi driver and took all bus numbers it wants.

> i2c-kempld.c
...
> how do you like it?

Not much. If we change kempld, then another driver might come along next
time and we have the same issue again. I want a generic solution.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c-eg20t: use dynamically registered adapter number

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]


> If not, it may make sense to add a helper function exposing
> __i2c_first_dynamic_bus_num to drivers (something like
> i2c_is_dynamic_bus_num().) After all, i2c_add_numbered_adapter() mostly
> makes sense if static i2c device definitions exist. If not,
> i2c_add_adapter() is just as good. So something like:
> 
> 	if (i2c_is_dynamic_bus_num(i))
> 		ret = i2c_add_adapter(pch_adap);
> 	else {
> 		pch_adap->nr = i;
> 		ret = i2c_add_numbered_adapter(pch_adap);
> 	}
> 
> may make sense. Unless someone has a better idea.

PASEMI does:

        smbus->adapter.nr = PCI_FUNC(dev->devfn);

I am unsure if there is any guarantee in what order PCI_FUNCs are
probed, yet I have the feeling we could try a little harder to get the
numbered adapter. What about this (untested, just to get the idea)?

static inline int i2c_add_adapter_try_numbered(struct i2c_adapter *new_adap)
{
	int ret;
	struct i2c_adapter *old_adap = i2c_get_adapter(new_adap->nr);

	if (old_adap && new_adap->nr >= __i2c_first_dynamic_bus_num) {
		i2c_put_adapter(old_adap);
		dev_dbg(&new_adap->dev, "Static bus number occupied, trying dynamic number\n");
		ret = i2c_add_adapter(new_adap);
	} else {
		ret = i2c_add_numbered_adapter(new_adap);
	}

	return ret;
}

I used 'static inline' because I think the drivers needing this should
carry the extra weight. But no major objection to put sth like this also
into the core. The documentation for this function should carry a big
note that this is only a workaround and it should not be used directly.

Thoughts?

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c-eg20t: use dynamically registered adapter number

[Jean Delvare]

Hi Wolfram,

On Sun, 18 Sep 2016 21:22:50 +0200, Wolfram Sang wrote:
> 
> > If not, it may make sense to add a helper function exposing
> > __i2c_first_dynamic_bus_num to drivers (something like
> > i2c_is_dynamic_bus_num().) After all, i2c_add_numbered_adapter() mostly
> > makes sense if static i2c device definitions exist. If not,
> > i2c_add_adapter() is just as good. So something like:
> > 
> > 	if (i2c_is_dynamic_bus_num(i))
> > 		ret = i2c_add_adapter(pch_adap);
> > 	else {
> > 		pch_adap->nr = i;
> > 		ret = i2c_add_numbered_adapter(pch_adap);
> > 	}
> > 
> > may make sense. Unless someone has a better idea.
> 
> PASEMI does:
> 
>         smbus->adapter.nr = PCI_FUNC(dev->devfn);
> 
> I am unsure if there is any guarantee in what order PCI_FUNCs are
> probed, yet I have the feeling we could try a little harder to get the
> numbered adapter. What about this (untested, just to get the idea)?
> 
> static inline int i2c_add_adapter_try_numbered(struct i2c_adapter *new_adap)
> {
> 	int ret;
> 	struct i2c_adapter *old_adap = i2c_get_adapter(new_adap->nr);
> 
> 	if (old_adap && new_adap->nr >= __i2c_first_dynamic_bus_num) {

Note that __i2c_first_dynamic_bus_num is not currently available to
device driver, so your function can't actually be inlined. This is the
reason why I proposed to introduce i2c_is_dynamic_bus_num(). If you
prefer to expose __i2c_first_dynamic_bus_num directly instead, that's
fine with me, but then you probably want to rename it.

> 		i2c_put_adapter(old_adap);
> 		dev_dbg(&new_adap->dev, "Static bus number occupied, trying dynamic number\n");
> 		ret = i2c_add_adapter(new_adap);
> 	} else {
> 		ret = i2c_add_numbered_adapter(new_adap);

You may be leaking a reference to old_adap here (if old_adap is set but
new_adap->nr < __i2c_first_dynamic_bus_num.)

> 	}
> 
> 	return ret;
> }

I'm a bit confused by the logic, it seems unnecessarily complex to me
(but please keep in mind I am working in a noisy environment these days
so maybe it's just me.)

If old_adap is set, i2c_add_numbered_adapter() has no chance of
working. So why are you additionally comparing with
__i2c_first_dynamic_bus_num? For me you should check either, not both.

My own proposal was checking __i2c_first_dynamic_bus_num. To be honest
I can't see the added value of relying on i2c_get_adapter() instead.
Would the result not be exactly the same? Plus it seems racy, just
because i2c_get_adapter() returns NULL at one point in time doesn't
mean the bus numbers will not have been assigned by the time you call
i2c_add_numbered_adapter().

> I used 'static inline' because I think the drivers needing this should
> carry the extra weight. But no major objection to put sth like this also
> into the core. The documentation for this function should carry a big
> note that this is only a workaround and it should not be used directly.

I don't care much where it goes, to be honest.

If you consider it a workaround, what would be the "real fix" for you?

I was wondering if selecting one of these drivers could set a Kconfig
option to initialize __i2c_first_dynamic_bus_num to a non-zero value.
Unfortunately there does not seem to be a way to set a numeric value to
a Kconfig option using select. We would have to do it indirectly as
with CONFIG_HZ:

choice
	default I2C_RESERVED_BUS_NR_0
	config I2C_RESERVED_BUS_NR_0
	config I2C_RESERVED_BUS_NR_2
endchoice

config I2C_RESERVED_BUS_NR
	int
	default 0 if I2C_RESERVED_BUS_NR_0
	default 2 if I2C_RESERVED_BUS_NR_2

config I2C_EG20T
	tristate "Intel EG20T PCH/LAPIS Semicon IOH(ML7213/ML7223/ML7831) I2C"
	depends on PCI && (X86_32 || MIPS || COMPILE_TEST)
	select I2C_RESERVED_BUS_NR_2

And in i2c-core.c:

__i2c_first_dynamic_bus_num = I2C_RESERVED_BUS_NR;

If that's possible at all... I'm not sure if select works on choice
config options.

Alternatively we could set the default directly based on driver
selection:

config I2C_RESERVED_BUS_NR
	int
	default 0
	default 2 if I2C_EG20T

This is more simple but a little harder to maintain. One possible
problem is if the number of buses isn't known at build time but could
change depending on the hardware, for example. Also I don't know if
more than one such driver can be included in the same kernel.

Or we can make it a user-visible option and leave it on whoever
configures the kernel to get it right. In all cases it would move the
decision to build-time instead of being set dynamically at run-time. 

Note that I am not claiming this is necessarily better than my initial
proposal. I just wanted to mention the possibility.

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] i2c-eg20t: use dynamically registered adapter number

[Jean Delvare]

Hi all,

On Mon, 19 Sep 2016 11:02:47 +0200, Jean Delvare wrote:
> My own proposal was checking __i2c_first_dynamic_bus_num. To be honest
> I can't see the added value of relying on i2c_get_adapter() instead.
> Would the result not be exactly the same? Plus it seems racy, just
> because i2c_get_adapter() returns NULL at one point in time doesn't
> mean the bus numbers will not have been assigned by the time you call
> i2c_add_numbered_adapter().
> (...)
> If you consider it a workaround, what would be the "real fix" for you?
> 
> I was wondering if selecting one of these drivers could set a Kconfig
> option to initialize __i2c_first_dynamic_bus_num to a non-zero value.
> Unfortunately there does not seem to be a way to set a numeric value to
> a Kconfig option using select. We would have to do it indirectly as
> with CONFIG_HZ:
> 
> choice
> 	default I2C_RESERVED_BUS_NR_0
> 	config I2C_RESERVED_BUS_NR_0
> 	config I2C_RESERVED_BUS_NR_2
> endchoice
> 
> config I2C_RESERVED_BUS_NR
> 	int
> 	default 0 if I2C_RESERVED_BUS_NR_0
> 	default 2 if I2C_RESERVED_BUS_NR_2
> 
> config I2C_EG20T
> 	tristate "Intel EG20T PCH/LAPIS Semicon IOH(ML7213/ML7223/ML7831) I2C"
> 	depends on PCI && (X86_32 || MIPS || COMPILE_TEST)
> 	select I2C_RESERVED_BUS_NR_2
> 
> And in i2c-core.c:
> 
> __i2c_first_dynamic_bus_num = I2C_RESERVED_BUS_NR;
> 
> If that's possible at all... I'm not sure if select works on choice
> config options.
> 
> Alternatively we could set the default directly based on driver
> selection:
> 
> config I2C_RESERVED_BUS_NR
> 	int
> 	default 0
> 	default 2 if I2C_EG20T
> 
> This is more simple but a little harder to maintain. One possible
> problem is if the number of buses isn't known at build time but could
> change depending on the hardware, for example. Also I don't know if
> more than one such driver can be included in the same kernel.
> 
> Or we can make it a user-visible option and leave it on whoever
> configures the kernel to get it right. In all cases it would move the
> decision to build-time instead of being set dynamically at run-time. 
> 
> Note that I am not claiming this is necessarily better than my initial
> proposal. I just wanted to mention the possibility.

I slept over it and start wondering if we aren't trying to address the
problem at the wrong end.

The initial reason why i2c_add_numbered_adapter() was used in the
i2c-eg20t driver (and, I suppose, i2c-pasemi and all other bus drivers)
is because the platform initialization code/data (native or OF-based)
may be declaring slave I2C devices on these buses. This bumps
__i2c_first_dynamic_bus_num, and the bus driver needs to call
i2c_add_numbered_adapter() to get bus numbers below that.

Maybe in that case the bus driver should ALSO get the static bus number
information from the platform initialization code/data. Then the bus
driver would check that information, and use it to call
i2c_add_numbered_adapter() as appropriate if present, or simply call
i2c_add_adapter() if not (or ask i2c_add_numbered_adapter for bus
number -1, which has the same result.)

Looks like drivers i2c-s3c2410, i2c-designware-*, i2c-kempld and
i2c-pxa are already getting this right. So maybe we don't need to
introduce another mechanism, but instead use what is already in place?

-- 
Jean Delvare
SUSE L3 Support

[PATCH] i2c-eg20t: use dynamically registered adapter number

[Yadi Hu]

From: Hu Yadi <Yadi.hu@windriver.com>

The eg20t driver uses i2c_add_numbered_adapter() to register adapter:

    pch_adap->nr = i;
    ret = i2c_add_numbered_adapter(pch_adap);

Variable i is assigned to 0, it means that i2c_eg20t is the first adapter
by default. if another adapter registers before eg20t, above code return
error for index conflict:

    i2c_eg20t 0000:05:0c.2: pch_i2c_probe :i2c_add_adapter[ch:0] FAILED
    i2c_eg20t: probe of 0000:05:0c.2 failed with error -16

So, we can replace i2c_add_numbered_adapter() with i2c_add_adapter()
interface.since it dynamically allocates the index number.

Signed-off-by: Hu Yadi <Yadi.hu@windriver.com>
---
 drivers/i2c/busses/i2c-eg20t.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-eg20t.c b/drivers/i2c/busses/i2c-eg20t.c
index 7a51ddc..2f4c2af 100644
--- a/drivers/i2c/busses/i2c-eg20t.c
+++ b/drivers/i2c/busses/i2c-eg20t.c
@@ -913,8 +913,7 @@ static int __devinit pch_i2c_probe(struct pci_dev *pdev,
 
 		pch_i2c_init(&adap_info->pch_data[i]);
 
-		pch_adap->nr = i;
-		ret = i2c_add_numbered_adapter(pch_adap);
+               ret = i2c_add_adapter(pch_adap);
 		if (ret) {
 			pch_pci_err(pdev, "i2c_add_adapter[ch:%d] FAILED\n", i);
 			goto err_add_adapter;
-- 
1.7.5.4