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

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

[David Brownell]

Update powerpc platforms to support new-style rtc-ds1307 driver:

 - Make i2c-mpc driver use numbered adapter;
 - Update boards identified by defconfig to list a ds1339 device:
    * mpc834x_itx
    * mpc8313_rdb

Didn't update the pasemi reference board; public info doesn't say
which chip it uses.

UNTESTED

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
NOTE:  cc'd the relevant powerpc maintainers lists; patches 1-3
will be archived in the rtc and i2c lists.  I'd like to see these
get tested/fixed so that the rtc-ds1307 driver can swich completely
to use the new-style I2c driver model.

 arch/powerpc/platforms/83xx/mpc8313_rdb.c |   11 +++++++++++
 arch/powerpc/platforms/83xx/mpc834x_itx.c |   12 ++++++++++++
 drivers/i2c/busses/i2c-mpc.c              |    3 ++-
 3 files changed, 25 insertions(+), 1 deletion(-)

--- 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;
 	}
--- at91.orig/arch/powerpc/platforms/83xx/mpc834x_itx.c	2007-05-12 23:31:24.000000000 -0700
+++ at91/arch/powerpc/platforms/83xx/mpc834x_itx.c	2007-05-13 11:22:36.000000000 -0700
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/seq_file.h>
 #include <linux/root_dev.h>
+#include <linux/i2c.h>
 
 #include <asm/system.h>
 #include <asm/atomic.h>
@@ -48,6 +49,15 @@ unsigned long isa_mem_base = 0;
  * Setup the architecture
  *
  */
+
+static struct i2c_board_info __initdata mpc834x_itx_i2c_devices[] = {
+	{ I2C_BOARD_INFO("rtc-ds1307", 0x68),
+	  .type = "ds1339",
+	  /* REVISIT .irq = IRQ2 ... this RTC has an alarm */
+	},
+	/* REVISIT: also has an eeprom */
+};
+
 static void __init mpc834x_itx_setup_arch(void)
 {
 #ifdef CONFIG_PCI
@@ -63,6 +73,8 @@ static void __init mpc834x_itx_setup_arc
 
 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
 #endif
+	i2c_register_board_info(0, mpc834x_itx_i2c_devices,
+			ARRAY_SIZE(mpc834x_itx_i2c_devices));
 }
 
 static void __init mpc834x_itx_init_IRQ(void)
--- at91.orig/arch/powerpc/platforms/83xx/mpc8313_rdb.c	2007-05-13 11:20:25.000000000 -0700
+++ at91/arch/powerpc/platforms/83xx/mpc8313_rdb.c	2007-05-13 11:21:49.000000000 -0700
@@ -14,6 +14,7 @@
  */
 
 #include <linux/pci.h>
+#include <linux/i2c.h>
 
 #include <asm/time.h>
 #include <asm/ipic.h>
@@ -38,6 +39,14 @@ unsigned long isa_mem_base = 0;
  * Setup the architecture
  *
  */
+static struct i2c_board_info __initdata mpc8313_rdb_i2c_devices[] = {
+	{ I2C_BOARD_INFO("rtc-ds1307", 0x68),
+	  .type = "ds1339",
+	  /* REVISIT .irq = IRQ4 ... this RTC has an alarm */
+	},
+	/* REVISIT: also has an eeprom and lm75 */
+};
+
 static void __init mpc8313_rdb_setup_arch(void)
 {
 #ifdef CONFIG_PCI
@@ -53,6 +62,8 @@ static void __init mpc8313_rdb_setup_arc
 
 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
 #endif
+	i2c_register_board_info(0, mpc8313_i2c_devices,
+			ARRAY_SIZE(mpc8313_i2c_devices));
 }
 
 void __init mpc8313_rdb_init_IRQ(void)

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

[Grant Likely]

On 5/14/07, David Brownell <david-b@pacbell.net> wrote:
> Update powerpc platforms to support new-style rtc-ds1307 driver:
>
>  - Make i2c-mpc driver use numbered adapter;
>  - Update boards identified by defconfig to list a ds1339 device:
>     * mpc834x_itx
>     * mpc8313_rdb
>
> Didn't update the pasemi reference board; public info doesn't say
> which chip it uses.
>
> UNTESTED
>
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
> NOTE:  cc'd the relevant powerpc maintainers lists; patches 1-3
> will be archived in the rtc and i2c lists.  I'd like to see these
> get tested/fixed so that the rtc-ds1307 driver can swich completely
> to use the new-style I2c driver model.
>
>  arch/powerpc/platforms/83xx/mpc8313_rdb.c |   11 +++++++++++
>  arch/powerpc/platforms/83xx/mpc834x_itx.c |   12 ++++++++++++
>  drivers/i2c/busses/i2c-mpc.c              |    3 ++-
>  3 files changed, 25 insertions(+), 1 deletion(-)
>
> --- 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.  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;

Cheers,
g.


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

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

[David Brownell]

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

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

[David Brownell]

> > > @@ -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 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 ...

Oh, and by the way:  patches updating I2C adapter drivers to work
with new-style drivers look to be OK to get into 2.6.22 with just
an Ack from Jean, via the MM tree.  (Right, Jean?)

In this case I'll hope someone else does the real submission, after
testing.  I just wanted to nudge the process along.

- Dave

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

[Grant Likely]

On 5/14/07, David Brownell <david-b@pacbell.net> wrote:
>
> > > > @@ -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 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 ...
>
> Oh, and by the way:  patches updating I2C adapter drivers to work
> with new-style drivers look to be OK to get into 2.6.22 with just
> an Ack from Jean, via the MM tree.  (Right, Jean?)
>
> In this case I'll hope someone else does the real submission, after
> testing.  I just wanted to nudge the process along

I'll submit this one (at least the i2c-mpc.c bits).  It's already
working on my mpc5200 board.  I don't have any of the other boards, so
those fixups should come from others.

Cheers,
g.

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

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

[Jean Delvare]

On Mon, 14 May 2007 11:49:27 -0700, David Brownell wrote:
> 
> > > > @@ -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 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 ...
> 
> Oh, and by the way:  patches updating I2C adapter drivers to work
> with new-style drivers look to be OK to get into 2.6.22 with just
> an Ack from Jean, via the MM tree.  (Right, Jean?)

Correct. I will happily ack such patches, but I won't be pushing them
upstream - unless waiting until 2.6.23 is OK for you.

-- 
Jean Delvare

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

[Olof Johansson]

On Mon, May 14, 2007 at 09:46:41AM -0700, David Brownell wrote:
> Update powerpc platforms to support new-style rtc-ds1307 driver:
> 
>  - Make i2c-mpc driver use numbered adapter;
>  - Update boards identified by defconfig to list a ds1339 device:
>     * mpc834x_itx
>     * mpc8313_rdb
> 
> Didn't update the pasemi reference board; public info doesn't say
> which chip it uses.

ds1338.

> NOTE:  cc'd the relevant powerpc maintainers lists; patches 1-3
> will be archived in the rtc and i2c lists.  I'd like to see these
> get tested/fixed so that the rtc-ds1307 driver can swich completely
> to use the new-style I2c driver model.

Specific URLs can sometimes be useful, so we don't have to go digging
through the archives. But thanks for the heads up, I'll make the
corresponding changes to my code and test it out.

(I'm guessing long-term, we will want to pull the chip info out of the
device tree instad of have it in a table in the board code, but that's
extra credit at the moment.)


-Olof

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

[David Brownell]

On Monday 14 May 2007, Olof Johansson wrote:
> On Mon, May 14, 2007 at 09:46:41AM -0700, David Brownell wrote:

> > Didn't update the pasemi reference board; public info doesn't say
> > which chip it uses.
> 
> ds1338.

OK.  The relevant updates should be evident from this patch.

Mr Grep tells me that if those powerpc platforms update along
with csb337, all current rtc-ds1307 users will be upgraded.

Then there's a separate issue of removing the old SENSORS_DS1337
driver.  Simpler IMO for those drivers to have one transition,
to new-style rtc-ds1307, not two (to old-style, then new-style).


> > NOTE:  cc'd the relevant powerpc maintainers lists; patches 1-3
> > will be archived in the rtc and i2c lists.  I'd like to see these
> > get tested/fixed so that the rtc-ds1307 driver can switch completely
> > to use the new-style I2c driver model.
> 
> Specific URLs can sometimes be useful, so we don't have to go digging
> through the archives. But thanks for the heads up, I'll make the
> corresponding changes to my code and test it out.

The URLS don't exist until after the email's gotten to the list
servers ... sort of a chicken/egg issue.

 
> (I'm guessing long-term, we will want to pull the chip info out of the
> device tree instad of have it in a table in the board code, but that's
> extra credit at the moment.)

For some definition of "long term".  ISTR seeing patches from Scott Wood
doing exactly that ... on the I2C list last November or so.  ;)

Right now my concern is how to migrate the rtc-ds1307 driver.  I'd
prefer to avoid needing to have a period where the legacy mode must
co-exist with the new-style driver support, or needing to entangle
with such tree conversions.

- Dave

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

[Alessandro Zummo]

On Mon, 14 May 2007 11:37:39 -0700
David Brownell <david-b@pacbell.net> wrote:

> 
> > (I'm guessing long-term, we will want to pull the chip info out of the
> > device tree instad of have it in a table in the board code, but that's
> > extra credit at the moment.)
> 
> For some definition of "long term".  ISTR seeing patches from Scott Wood
> doing exactly that ... on the I2C list last November or so.  ;)

  I saw them too. pretty nice set of patches. I would agree
 on them fully.


-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

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

[Olof Johansson]

On Mon, May 14, 2007 at 11:37:39AM -0700, David Brownell wrote:
> On Monday 14 May 2007, Olof Johansson wrote:
> > On Mon, May 14, 2007 at 09:46:41AM -0700, David Brownell wrote:
> 
> > > Didn't update the pasemi reference board; public info doesn't say
> > > which chip it uses.
> > 
> > ds1338.
> 
> OK.  The relevant updates should be evident from this patch.
> 
> Mr Grep tells me that if those powerpc platforms update along
> with csb337, all current rtc-ds1307 users will be upgraded.
> 
> Then there's a separate issue of removing the old SENSORS_DS1337
> driver.  Simpler IMO for those drivers to have one transition,
> to new-style rtc-ds1307, not two (to old-style, then new-style).

Sounds good.

> > > NOTE:  cc'd the relevant powerpc maintainers lists; patches 1-3
> > > will be archived in the rtc and i2c lists.  I'd like to see these
> > > get tested/fixed so that the rtc-ds1307 driver can switch completely
> > > to use the new-style I2c driver model.
> > 
> > Specific URLs can sometimes be useful, so we don't have to go digging
> > through the archives. But thanks for the heads up, I'll make the
> > corresponding changes to my code and test it out.
> 
> The URLS don't exist until after the email's gotten to the list
> servers ... sort of a chicken/egg issue.

Ah yes, good point.

> > (I'm guessing long-term, we will want to pull the chip info out of the
> > device tree instad of have it in a table in the board code, but that's
> > extra credit at the moment.)
> 
> For some definition of "long term".  ISTR seeing patches from Scott Wood
> doing exactly that ... on the I2C list last November or so.  ;)

Yep, I asked on IRC and got a similar answer. I had forgotten about them.

> Right now my concern is how to migrate the rtc-ds1307 driver.  I'd
> prefer to avoid needing to have a period where the legacy mode must
> co-exist with the new-style driver support, or needing to entangle
> with such tree conversions.

Which is why I said "long term", as in "after this is done". :)


-Olof