[i2c] Numbering of i2c adapters

[Grant Likely]

On 9/11/07, Jean Delvare <khali at linux-fr.org> wrote:
>
> Hi Rafal,
>
> On 9/11/2007, "Rafal Zabdyr" <armo at semihalf.com> wrote:
> >We found a problem which is connected with commit (2.6.23-rc5):
> >1469fa263870acd890a4b9f6ef557acc5d673b44 "i2c-mpc: Use
> >i2c_add_numbered_adapter"
> >
> >1. Our platform has CONFIG_PPC_MERGE=y so the following line don't compile:
> > i2c->adap.nr = pdev->id;
> >(file drivers/i2c/busses/i2c-mpc.c, function fsl_i2c_probe())
> >Moving the above statement out of CONFIG_PPC_MERGE allows to compile,
> >but there is still a problem with setting adap.nr:
>
> There is no CONFIG_PPC_MERGE in i2c-mpc in 2.6.23-rc5. What are you
> talking about? Presumably out-of-tree changes?

Rafal:  ?!?  i2c-mpc.c compiles fine for me with CONFIG_PPC_MERGE=y.

struct platform_device always has the .id member.  Ideally
arch/powerpc drivers should use the of_platform_bus, but in this case
the driver is still using platform_bus and an adapter layer
(fsl_i2c_of_init in arch/powerpc/sysdev/fsl_soc.c) populates the
platform_device structure with the appropriate info.

Jean: CONFIG_PPC_MERGE is a arch/ppc --> arch/powerpc thing.  It just
indicates that we're compiling an arch/powerpc kernel, not an arch/ppc
kernel.

>
> >2. In case of using two or more i2c buses, the
> >i2c_add_numbered_adapter() would fail
> >in the following section (if statement), because adap->nr is always 0:
> >
> >    status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
> >    if (status == 0 && id != adap->nr) {
> >        status = -EBUSY;
> >        idr_remove(&i2c_adapter_idr, id);
> >    }
>
> The code assumes that you have one different platform device for every
> i2c_adapter. If you have two or more i2c_adapters for just one platform
> device, then yes, it is going to fail.
>
> This is really up to each platform to come up with a coherent i2c bus
> numbering scheme. If the above change is not coherent with what the MPC
> platform (whatever that is) is doing, then of course we have to fix it.
> Grant, can you please clarify?

fsl_i2c_of_init enumerates the i2c controllers that it finds.
adap->nr should not always be 0.

>
> >3. Setting adap.nr was previously based on pdev->id which is not
> >available in the context of_device struct in the new I2C approach.
> >
> >Do you think it is a problem, or am I missing something?

of_device is *not* used by this driver.  (it probably should be, but
it isn't).  This is still a platform_device driver and fsl_i2c_of_init
registers the needed platform_device for each controller.

>
> I'm not familiar with the platform and of_device  that is going on on
> the PPC front, so I can't really tell. All that matters is that the i2c
> bus numbering model (i.e. how you compute the i2c bus number from some
> of the underlying device properties) is clear.
>
> --
> Jean Delvare
>


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely at secretlab.ca
(403) 399-0195