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

[Wolfram Sang <wsa@the-dreams.de>]

[-- 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 --]