Re: [patch/rft 4/4] rtc-ds1307 platform update, powerpc mpc834x_itx and mpc8313_rdb

[David Brownell <david-b@pacbell.net>]

On Monday 14 May 2007, Grant Likely wrote:
> > --- at91.orig/drivers/i2c/busses/i2c-mpc.c      2007-05-10 00:48:22.000000000 -0700
> > +++ at91/drivers/i2c/busses/i2c-mpc.c   2007-05-13 10:56:52.000000000 -0700
> > @@ -280,6 +280,7 @@ static struct i2c_adapter mpc_ops = {
> >         .owner = THIS_MODULE,
> >         .name = "MPC adapter",
> >         .id = I2C_HW_MPC107,
> > +       /* .nr = 0, */
> >         .algo = &mpc_algo,
> >         .class = I2C_CLASS_HWMON,
> >         .timeout = 1,
> > @@ -329,7 +330,7 @@ static int fsl_i2c_probe(struct platform
> >         i2c->adap = mpc_ops;
> >         i2c_set_adapdata(&i2c->adap, i2c);
> >         i2c->adap.dev.parent = &pdev->dev;
> > -       if ((result = i2c_add_adapter(&i2c->adap)) < 0) {
> > +       if ((result = i2c_add_numbered_adapter(&i2c->adap)) < 0) {
> >                 printk(KERN_ERR "i2c-mpc - failed to add adapter\n");
> >                 goto fail_add;
> >         }
> 
> I think this will break many boards as more than one i2c adapter is
> common on these chips. 

It didn't look like that from my quick glance at the sources,
but I could have been deceived.  I certainly don't know powerpc
hardware.


> If I read the code correctly, the first 
> adapter will be registered with adap->nr == 0.  The second adapter
> will also be registered with adap->nr == 0; but
> i2c_add_numbered_adapter will return -EBUSY because the id is already
> used.
> 
> I think you need to add this line before the call to i2c_add_numbered_adapter:
> 
> i2c->adap.nr = pdev->id;

If these chips do have multiple instances of that adapter logic,
that would be an appropriate change.  The only "gotcha" might be
that some platforms use the "pdev->id = -1" trick when there's
only a single instance ... that would probably map to zero.

I'll hope that whoever makes all this work will resolve those
particular questions ...

- Dave