Re: [PATCH] i2c-au1550: convert to platform driver

Re: [PATCH] i2c-au1550: convert to platform driver

[Jean Delvare]

Hi Manuel,

On Sat, 27 Oct 2007 20:05:21 +0200, Jean Delvare wrote:
> Hi Manuel,
> 
> On Sat, 6 Oct 2007 18:16:06 +0200, Manuel Lauss wrote:
> > From db75417225eb3bddcbc2e5b57d97fef2cc0ce4c6 Mon Sep 17 00:00:00 2001
> > From: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
> > Date: Sat, 6 Oct 2007 17:21:40 +0200
> > Subject: [PATCH] i2c-au1550: convert to platform driver
> > 
> > Convert the i2c-au1550.c driver to a platform_device, get rid of the
> > i2c-au1550.h file and hook up I2C for the alchemy demoboards.
> > 
> > Signed-off-by: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
> > ---
> >  arch/mips/au1000/common/platform.c |   21 +++++
> >  drivers/i2c/busses/i2c-au1550.c    |  152 ++++++++++++++++++++---------------
> >  drivers/i2c/busses/i2c-au1550.h    |   32 --------
> >  3 files changed, 108 insertions(+), 97 deletions(-)
> >  delete mode 100644 drivers/i2c/busses/i2c-au1550.h

Any news from this patch? I didn't hear about this since I reviewed
it back in October 2007. If you would submit an update addressing the
problems I had noticed back then, I could queue this patch for 2.6.25.
>From what I remember, there weren't many changes needed.

Thanks,
-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-au1550: convert to platform driver

[Manuel Lauss]

On Thu, Jan 17, 2008 at 04:59:16PM +0100, Jean Delvare wrote:
> Hi Manuel,
> 
> On Sat, 27 Oct 2007 20:05:21 +0200, Jean Delvare wrote:
> > Hi Manuel,
> > 
> > On Sat, 6 Oct 2007 18:16:06 +0200, Manuel Lauss wrote:
> > > From db75417225eb3bddcbc2e5b57d97fef2cc0ce4c6 Mon Sep 17 00:00:00 2001
> > > From: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
> > > Date: Sat, 6 Oct 2007 17:21:40 +0200
> > > Subject: [PATCH] i2c-au1550: convert to platform driver
> > > 
> > > Convert the i2c-au1550.c driver to a platform_device, get rid of the
> > > i2c-au1550.h file and hook up I2C for the alchemy demoboards.
> > > 
> > > Signed-off-by: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
> > > ---
> > >  arch/mips/au1000/common/platform.c |   21 +++++
> > >  drivers/i2c/busses/i2c-au1550.c    |  152 ++++++++++++++++++++---------------
> > >  drivers/i2c/busses/i2c-au1550.h    |   32 --------
> > >  3 files changed, 108 insertions(+), 97 deletions(-)
> > >  delete mode 100644 drivers/i2c/busses/i2c-au1550.h
> 
> Any news from this patch? I didn't hear about this since I reviewed
> it back in October 2007. If you would submit an update addressing the
> problems I had noticed back then, I could queue this patch for 2.6.25.
> From what I remember, there weren't many changes needed.

I was waiting for the other au1550 fix to be accepted before resending.
I'll post a revamped version shortly.

Thanks,
	Manuel Lauss

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-au1550: convert to platform driver

[Manuel Lauss]

Hi Jean,

On Sat, Oct 27, 2007 at 08:05:21PM +0200, Jean Delvare wrote:
> 
> > +/* All Alchemy demoboards with I2C have this #define in their headers */
> > +#ifdef SMBUS_PSC_BASE
> > +static struct resource pbdb_smbus_resources[] = {
> > +	{
> > +		.start	= SMBUS_PSC_BASE,
> > +		.end	= SMBUS_PSC_BASE + 0x24 - 1,
> > +		.flags	= IORESOURCE_MEM,
> 
> Aren't resources supposed to have a name, so that they are displayed
> properly in /proc/iomem)?

it looks okay to me the way it is, so I didn't change it:

b1a00000-b1a00023 : au1xpsc_smbus.0
  b1a00000-b1a00023 : au1xpsc_smbus


Thanks!

---
>From a7d9aff8c1140682d57c765653b018b53b8c5193 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
Date: Fri, 18 Jan 2008 08:38:46 +0100
Subject: [PATCH] [PATCH] i2c: convert i2c-au1550 to platform driver.

Converts the i2c-au1550 bus driver to platform driver and
registers a platform device for the Alchemy Pb/Db series of
boards.

Signed-off-by: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
---
 arch/mips/au1000/common/platform.c |   24 +++++
 drivers/i2c/busses/i2c-au1550.c    |  166 ++++++++++++++++++++++--------------
 drivers/i2c/busses/i2c-au1550.h    |   32 -------
 3 files changed, 126 insertions(+), 96 deletions(-)
 delete mode 100644 drivers/i2c/busses/i2c-au1550.h

diff --git a/arch/mips/au1000/common/platform.c b/arch/mips/au1000/common/platform.c
index d51e18f..ccc5ec8 100644
--- a/arch/mips/au1000/common/platform.c
+++ b/arch/mips/au1000/common/platform.c
@@ -270,6 +270,27 @@ static struct platform_device smc91x_device = {
 
 #endif
 
+/* All Alchemy demoboards with I2C have this #define in their headers */
+#ifdef SMBUS_PSC_BASE
+static struct resource pbdb_smbus_resources[] = {
+       {
+               .start  = SMBUS_PSC_BASE,
+               .end    = SMBUS_PSC_BASE + 0x24 - 1,
+               .flags  = IORESOURCE_MEM,
+       },
+};
+
+static struct platform_device pbdb_smbus_device = {
+	.dev = {
+		.platform_data = (void *)0,	/* bus number */
+	},
+       .name           = "au1xpsc_smbus",
+       .id             = 0,
+       .num_resources  = ARRAY_SIZE(pbdb_smbus_resources),
+       .resource       = pbdb_smbus_resources,
+};
+#endif
+
 static struct platform_device *au1xxx_platform_devices[] __initdata = {
 	&au1xxx_usb_ohci_device,
 	&au1x00_pcmcia_device,
@@ -287,6 +308,9 @@ static struct platform_device *au1xxx_platform_devices[] __initdata = {
 #ifdef CONFIG_MIPS_DB1200
 	&smc91x_device,
 #endif
+#ifdef SMBUS_PSC_BASE
+	&pbdb_smbus_device,
+#endif
 };
 
 int __init au1xxx_platform_init(void)
diff --git a/drivers/i2c/busses/i2c-au1550.c b/drivers/i2c/busses/i2c-au1550.c
index 7d51a43..93040c0 100644
--- a/drivers/i2c/busses/i2c-au1550.c
+++ b/drivers/i2c/busses/i2c-au1550.c
@@ -30,14 +30,22 @@
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/slab.h>
 
 #include <asm/mach-au1x00/au1xxx.h>
 #include <asm/mach-au1x00/au1xxx_psc.h>
 
-#include "i2c-au1550.h"
+struct i2c_au1550_data {
+	u32	psc_base;
+	int	xfer_timeout;
+	int	ack_timeout;
+	struct i2c_adapter adap;
+	struct resource *ioarea;
+};
 
 static int
 wait_xfer_done(struct i2c_au1550_data *adap)
@@ -299,18 +307,48 @@ static const struct i2c_algorithm au1550_algo = {
  * Prior to calling us, the 50MHz clock frequency and routing
  * must have been set up for the PSC indicated by the adapter.
  */
-int
-i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
+static int __devinit
+i2c_au1550_probe(struct platform_device *pdev)
 {
-	struct i2c_au1550_data *adap = i2c_adap->algo_data;
-	volatile psc_smb_t	*sp;
-	u32	stat;
+	struct i2c_au1550_data *priv;
+	volatile psc_smb_t *sp;
+	struct resource *r;
+	u32 stat;
+	int ret;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		ret = -ENODEV;
+		goto out;
+	}
 
-	i2c_adap->algo = &au1550_algo;
+	priv = kzalloc(sizeof(struct i2c_au1550_data), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	
+	priv->ioarea = request_mem_region(r->start, r->end - r->start + 1,
+					  pdev->name);
+	if (!priv->ioarea) {
+		ret = -EBUSY;
+		goto out_mem;
+	}
+
+	priv->psc_base = r->start;
+	priv->xfer_timeout = 200;
+	priv->ack_timeout = 200;
+
+	priv->adap.id = I2C_HW_AU1550_PSC;
+	priv->adap.nr = (int)pdev->dev.platform_data;
+	priv->adap.algo = &au1550_algo;
+	priv->adap.algo_data = priv;
+	priv->adap.dev.parent = &pdev->dev;
+	strlcpy(priv->adap.name, "Au1xxx PSC I2C", 14);
 
 	/* Now, set up the PSC for SMBus PIO mode.
 	*/
-	sp = (volatile psc_smb_t *)(adap->psc_base);
+	sp = (volatile psc_smb_t *)(priv->psc_base);
 	sp->psc_ctrl = PSC_CTRL_DISABLE;
 	au_sync();
 	sp->psc_sel = PSC_SEL_PS_SMBUSMODE;
@@ -348,87 +386,87 @@ i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
 		au_sync();
 	} while ((stat & PSC_SMBSTAT_DR) == 0);
 
-	return i2c_add_adapter(i2c_adap);
-}
+	ret = i2c_add_numbered_adapter(&priv->adap);
+	if (ret == 0) {
+		platform_set_drvdata(pdev, priv);
+		return 0;
+	}
+
+	/* disable the PSC */
+	sp->psc_smbcfg = 0;
+	sp->psc_ctrl = PSC_CTRL_DISABLE;
+	au_sync();
 
+	release_resource(priv->ioarea);
+	kfree(priv->ioarea);
+out_mem:
+	kfree(priv);
+out:
+	return ret;
+}
 
-int
-i2c_au1550_del_bus(struct i2c_adapter *adap)
+static int __devexit
+i2c_au1550_remove(struct platform_device *pdev)
 {
-	return i2c_del_adapter(adap);
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	psc_smb_t *sp = (psc_smb_t *)priv->psc_base;
+
+	platform_set_drvdata(pdev, NULL);
+	i2c_del_adapter(&priv->adap);
+	sp->psc_smbcfg = 0;
+	sp->psc_ctrl = PSC_CTRL_DISABLE;
+	au_sync();
+	release_resource(priv->ioarea);
+	kfree(priv->ioarea);	
+	kfree(priv);
+	return 0;
 }
 
 static int
-pb1550_reg(struct i2c_client *client)
+i2c_au1550_suspend(struct platform_device *pdev, pm_message_t state)
 {
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	psc_smb_t *sp = (psc_smb_t *)priv->psc_base;
+
+	sp->psc_ctrl = PSC_CTRL_SUSPEND;
+	au_sync();
 	return 0;
 }
 
 static int
-pb1550_unreg(struct i2c_client *client)
+i2c_au1550_resume(struct platform_device *pdev)
 {
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	psc_smb_t *sp = (psc_smb_t *)priv->psc_base;
+
+	sp->psc_ctrl = PSC_CTRL_ENABLE;
+	au_sync();
+	while (!(sp->psc_smbstat & PSC_SMBSTAT_SR))
+		au_sync();
 	return 0;
 }
 
-static struct i2c_au1550_data pb1550_i2c_info = {
-	SMBUS_PSC_BASE, 200, 200
-};
-
-static struct i2c_adapter pb1550_board_adapter = {
-	name:              "pb1550 adapter",
-	id:                I2C_HW_AU1550_PSC,
-	algo:              NULL,
-	algo_data:         &pb1550_i2c_info,
-	client_register:   pb1550_reg,
-	client_unregister: pb1550_unreg,
+static struct platform_driver au1xpsc_smbus_driver = {
+	.driver = {
+		.name	= "au1xpsc_smbus",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= i2c_au1550_probe,
+	.remove		= __devexit_p(i2c_au1550_remove),
+	.suspend	= i2c_au1550_suspend,
+	.resume		= i2c_au1550_resume,
 };
 
-/* BIG hack to support the control interface on the Wolfson WM8731
- * audio codec on the Pb1550 board.  We get an address and two data
- * bytes to write, create an i2c message, and send it across the
- * i2c transfer function.  We do this here because we have access to
- * the i2c adapter structure.
- */
-static struct i2c_msg wm_i2c_msg;  /* We don't want this stuff on the stack */
-static	u8 i2cbuf[2];
-
-int
-pb1550_wm_codec_write(u8 addr, u8 reg, u8 val)
-{
-	wm_i2c_msg.addr = addr;
-	wm_i2c_msg.flags = 0;
-	wm_i2c_msg.buf = i2cbuf;
-	wm_i2c_msg.len = 2;
-	i2cbuf[0] = reg;
-	i2cbuf[1] = val;
-
-	return pb1550_board_adapter.algo->master_xfer(&pb1550_board_adapter, &wm_i2c_msg, 1);
-}
-
 static int __init
 i2c_au1550_init(void)
 {
-	printk(KERN_INFO "Au1550 I2C: ");
-
-	/* This is where we would set up a 50MHz clock source
-	 * and routing.  On the Pb1550, the SMBus is PSC2, which
-	 * uses a shared clock with USB.  This has been already
-	 * configured by Yamon as a 48MHz clock, close enough
-	 * for our work.
-	 */
-        if (i2c_au1550_add_bus(&pb1550_board_adapter) < 0) {
-		printk("failed to initialize.\n");
-                return -ENODEV;
-	}
-
-	printk("initialized.\n");
-	return 0;
+	return platform_driver_register(&au1xpsc_smbus_driver);
 }
 
 static void __exit
 i2c_au1550_exit(void)
 {
-	i2c_au1550_del_bus(&pb1550_board_adapter);
+	platform_driver_unregister(&au1xpsc_smbus_driver);
 }
 
 MODULE_AUTHOR("Dan Malek, Embedded Edge, LLC.");
diff --git a/drivers/i2c/busses/i2c-au1550.h b/drivers/i2c/busses/i2c-au1550.h
deleted file mode 100644
index fce15d1..0000000
--- a/drivers/i2c/busses/i2c-au1550.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Copyright (C) 2004 Embedded Edge, LLC <dan-L1vi/lXTdtu4JprhXtzr+Q@public.gmane.org>
- * 2.6 port by Matt Porter <mporter-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-
-#ifndef I2C_AU1550_H
-#define I2C_AU1550_H
-
-struct i2c_au1550_data {
-	u32	psc_base;
-	int	xfer_timeout;
-	int	ack_timeout;
-};
-
-int i2c_au1550_add_bus(struct i2c_adapter *);
-int i2c_au1550_del_bus(struct i2c_adapter *);
-
-#endif /* I2C_AU1550_H */
-- 
1.5.3.7


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-au1550: convert to platform driver

[Jean Delvare]

Hi Manuel,

On Fri, 18 Jan 2008 10:09:13 +0100, Manuel Lauss wrote:
> Hi Jean,
> 
> On Sat, Oct 27, 2007 at 08:05:21PM +0200, Jean Delvare wrote:
> > 
> > > +/* All Alchemy demoboards with I2C have this #define in their headers */
> > > +#ifdef SMBUS_PSC_BASE
> > > +static struct resource pbdb_smbus_resources[] = {
> > > +	{
> > > +		.start	= SMBUS_PSC_BASE,
> > > +		.end	= SMBUS_PSC_BASE + 0x24 - 1,
> > > +		.flags	= IORESOURCE_MEM,
> > 
> > Aren't resources supposed to have a name, so that they are displayed
> > properly in /proc/iomem)?
> 
> it looks okay to me the way it is, so I didn't change it:
> 
> b1a00000-b1a00023 : au1xpsc_smbus.0
>   b1a00000-b1a00023 : au1xpsc_smbus

Ah, OK, I didn't know that. I guess that the driver core can give
default names to resources that have no name at registration time.
Thanks for the info!

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-au1550: convert to platform driver

[Manuel Lauss]

Jean,

Here's a new diff which makes checkpatch happy, intended to be 
applied on top of the zero-byte fix.

Thanks!
	Manuel Lauss

---
>From be82598e26a13363fda3e14fcc67c45e6734fd07 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
Date: Fri, 18 Jan 2008 09:48:52 +0100
Subject: [PATCH] [PATCH] i2c: convert i2c-au1550 to platform driver.

Converts the i2c-au1550 bus driver to platform driver and
registers a platform device for the Alchemy Pb/Db series of
boards.

Signed-off-by: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
---
 arch/mips/au1000/common/platform.c |   24 +++++
 drivers/i2c/busses/i2c-au1550.c    |  166 ++++++++++++++++++++++--------------
 drivers/i2c/busses/i2c-au1550.h    |   32 -------
 3 files changed, 126 insertions(+), 96 deletions(-)
 delete mode 100644 drivers/i2c/busses/i2c-au1550.h

diff --git a/arch/mips/au1000/common/platform.c b/arch/mips/au1000/common/platform.c
index d51e18f..5f47674 100644
--- a/arch/mips/au1000/common/platform.c
+++ b/arch/mips/au1000/common/platform.c
@@ -270,6 +270,27 @@ static struct platform_device smc91x_device = {
 
 #endif
 
+/* All Alchemy demoboards with I2C have this #define in their headers */
+#ifdef SMBUS_PSC_BASE
+static struct resource pbdb_smbus_resources[] = {
+	{
+		.start  = SMBUS_PSC_BASE,
+		.end    = SMBUS_PSC_BASE + 0x24 - 1,
+		.flags  = IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device pbdb_smbus_device = {
+	.dev = {
+		.platform_data = (void *)0,	/* bus number */
+	},
+	.name           = "au1xpsc_smbus",
+	.id             = 0,
+	.num_resources  = ARRAY_SIZE(pbdb_smbus_resources),
+	.resource       = pbdb_smbus_resources,
+};
+#endif
+
 static struct platform_device *au1xxx_platform_devices[] __initdata = {
 	&au1xxx_usb_ohci_device,
 	&au1x00_pcmcia_device,
@@ -287,6 +308,9 @@ static struct platform_device *au1xxx_platform_devices[] __initdata = {
 #ifdef CONFIG_MIPS_DB1200
 	&smc91x_device,
 #endif
+#ifdef SMBUS_PSC_BASE
+	&pbdb_smbus_device,
+#endif
 };
 
 int __init au1xxx_platform_init(void)
diff --git a/drivers/i2c/busses/i2c-au1550.c b/drivers/i2c/busses/i2c-au1550.c
index 7d51a43..801b00c 100644
--- a/drivers/i2c/busses/i2c-au1550.c
+++ b/drivers/i2c/busses/i2c-au1550.c
@@ -30,14 +30,22 @@
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/slab.h>
 
 #include <asm/mach-au1x00/au1xxx.h>
 #include <asm/mach-au1x00/au1xxx_psc.h>
 
-#include "i2c-au1550.h"
+struct i2c_au1550_data {
+	u32	psc_base;
+	int	xfer_timeout;
+	int	ack_timeout;
+	struct i2c_adapter adap;
+	struct resource *ioarea;
+};
 
 static int
 wait_xfer_done(struct i2c_au1550_data *adap)
@@ -299,18 +307,48 @@ static const struct i2c_algorithm au1550_algo = {
  * Prior to calling us, the 50MHz clock frequency and routing
  * must have been set up for the PSC indicated by the adapter.
  */
-int
-i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
+static int __devinit
+i2c_au1550_probe(struct platform_device *pdev)
 {
-	struct i2c_au1550_data *adap = i2c_adap->algo_data;
-	volatile psc_smb_t	*sp;
-	u32	stat;
+	struct i2c_au1550_data *priv;
+	psc_smb_t *sp;
+	struct resource *r;
+	u32 stat;
+	int ret;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		ret = -ENODEV;
+		goto out;
+	}
 
-	i2c_adap->algo = &au1550_algo;
+	priv = kzalloc(sizeof(struct i2c_au1550_data), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	priv->ioarea = request_mem_region(r->start, r->end - r->start + 1,
+					  pdev->name);
+	if (!priv->ioarea) {
+		ret = -EBUSY;
+		goto out_mem;
+	}
+
+	priv->psc_base = r->start;
+	priv->xfer_timeout = 200;
+	priv->ack_timeout = 200;
+
+	priv->adap.id = I2C_HW_AU1550_PSC;
+	priv->adap.nr = (int)pdev->dev.platform_data;
+	priv->adap.algo = &au1550_algo;
+	priv->adap.algo_data = priv;
+	priv->adap.dev.parent = &pdev->dev;
+	strlcpy(priv->adap.name, "Au1xxx PSC I2C", 14);
 
 	/* Now, set up the PSC for SMBus PIO mode.
 	*/
-	sp = (volatile psc_smb_t *)(adap->psc_base);
+	sp = (psc_smb_t *)priv->psc_base;
 	sp->psc_ctrl = PSC_CTRL_DISABLE;
 	au_sync();
 	sp->psc_sel = PSC_SEL_PS_SMBUSMODE;
@@ -348,87 +386,87 @@ i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
 		au_sync();
 	} while ((stat & PSC_SMBSTAT_DR) == 0);
 
-	return i2c_add_adapter(i2c_adap);
-}
+	ret = i2c_add_numbered_adapter(&priv->adap);
+	if (ret == 0) {
+		platform_set_drvdata(pdev, priv);
+		return 0;
+	}
+
+	/* disable the PSC */
+	sp->psc_smbcfg = 0;
+	sp->psc_ctrl = PSC_CTRL_DISABLE;
+	au_sync();
 
+	release_resource(priv->ioarea);
+	kfree(priv->ioarea);
+out_mem:
+	kfree(priv);
+out:
+	return ret;
+}
 
-int
-i2c_au1550_del_bus(struct i2c_adapter *adap)
+static int __devexit
+i2c_au1550_remove(struct platform_device *pdev)
 {
-	return i2c_del_adapter(adap);
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	psc_smb_t *sp = (psc_smb_t *)priv->psc_base;
+
+	platform_set_drvdata(pdev, NULL);
+	i2c_del_adapter(&priv->adap);
+	sp->psc_smbcfg = 0;
+	sp->psc_ctrl = PSC_CTRL_DISABLE;
+	au_sync();
+	release_resource(priv->ioarea);
+	kfree(priv->ioarea);
+	kfree(priv);
+	return 0;
 }
 
 static int
-pb1550_reg(struct i2c_client *client)
+i2c_au1550_suspend(struct platform_device *pdev, pm_message_t state)
 {
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	psc_smb_t *sp = (psc_smb_t *)priv->psc_base;
+
+	sp->psc_ctrl = PSC_CTRL_SUSPEND;
+	au_sync();
 	return 0;
 }
 
 static int
-pb1550_unreg(struct i2c_client *client)
+i2c_au1550_resume(struct platform_device *pdev)
 {
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	psc_smb_t *sp = (psc_smb_t *)priv->psc_base;
+
+	sp->psc_ctrl = PSC_CTRL_ENABLE;
+	au_sync();
+	while (!(sp->psc_smbstat & PSC_SMBSTAT_SR))
+		au_sync();
 	return 0;
 }
 
-static struct i2c_au1550_data pb1550_i2c_info = {
-	SMBUS_PSC_BASE, 200, 200
-};
-
-static struct i2c_adapter pb1550_board_adapter = {
-	name:              "pb1550 adapter",
-	id:                I2C_HW_AU1550_PSC,
-	algo:              NULL,
-	algo_data:         &pb1550_i2c_info,
-	client_register:   pb1550_reg,
-	client_unregister: pb1550_unreg,
+static struct platform_driver au1xpsc_smbus_driver = {
+	.driver = {
+		.name	= "au1xpsc_smbus",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= i2c_au1550_probe,
+	.remove		= __devexit_p(i2c_au1550_remove),
+	.suspend	= i2c_au1550_suspend,
+	.resume		= i2c_au1550_resume,
 };
 
-/* BIG hack to support the control interface on the Wolfson WM8731
- * audio codec on the Pb1550 board.  We get an address and two data
- * bytes to write, create an i2c message, and send it across the
- * i2c transfer function.  We do this here because we have access to
- * the i2c adapter structure.
- */
-static struct i2c_msg wm_i2c_msg;  /* We don't want this stuff on the stack */
-static	u8 i2cbuf[2];
-
-int
-pb1550_wm_codec_write(u8 addr, u8 reg, u8 val)
-{
-	wm_i2c_msg.addr = addr;
-	wm_i2c_msg.flags = 0;
-	wm_i2c_msg.buf = i2cbuf;
-	wm_i2c_msg.len = 2;
-	i2cbuf[0] = reg;
-	i2cbuf[1] = val;
-
-	return pb1550_board_adapter.algo->master_xfer(&pb1550_board_adapter, &wm_i2c_msg, 1);
-}
-
 static int __init
 i2c_au1550_init(void)
 {
-	printk(KERN_INFO "Au1550 I2C: ");
-
-	/* This is where we would set up a 50MHz clock source
-	 * and routing.  On the Pb1550, the SMBus is PSC2, which
-	 * uses a shared clock with USB.  This has been already
-	 * configured by Yamon as a 48MHz clock, close enough
-	 * for our work.
-	 */
-        if (i2c_au1550_add_bus(&pb1550_board_adapter) < 0) {
-		printk("failed to initialize.\n");
-                return -ENODEV;
-	}
-
-	printk("initialized.\n");
-	return 0;
+	return platform_driver_register(&au1xpsc_smbus_driver);
 }
 
 static void __exit
 i2c_au1550_exit(void)
 {
-	i2c_au1550_del_bus(&pb1550_board_adapter);
+	platform_driver_unregister(&au1xpsc_smbus_driver);
 }
 
 MODULE_AUTHOR("Dan Malek, Embedded Edge, LLC.");
diff --git a/drivers/i2c/busses/i2c-au1550.h b/drivers/i2c/busses/i2c-au1550.h
deleted file mode 100644
index fce15d1..0000000
--- a/drivers/i2c/busses/i2c-au1550.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Copyright (C) 2004 Embedded Edge, LLC <dan-L1vi/lXTdtu4JprhXtzr+Q@public.gmane.org>
- * 2.6 port by Matt Porter <mporter-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-
-#ifndef I2C_AU1550_H
-#define I2C_AU1550_H
-
-struct i2c_au1550_data {
-	u32	psc_base;
-	int	xfer_timeout;
-	int	ack_timeout;
-};
-
-int i2c_au1550_add_bus(struct i2c_adapter *);
-int i2c_au1550_del_bus(struct i2c_adapter *);
-
-#endif /* I2C_AU1550_H */
-- 
1.5.3.7


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-au1550: convert to platform driver

[Manuel Lauss]

Hi again,

To make my humiliation complete, here's the third iteration
of this patch. The original authors put a lot of 'volatile's
in there for good reason; without them the driver hangs very
early. The previous iteration removed them because of checkpatch
complaining.

Thanks!
	Manuel Lauss
---

>From 9cf3ca9cc3663cc6cf74f3da52097cf998e8f121 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
Date: Fri, 18 Jan 2008 10:03:17 +0100
Subject: [PATCH] [PATCH] i2c: convert i2c-au1550 to platform driver #3

Convert the i2c-au1550 bus driver to platform driver, and
register a platform device for the Alchemy Db/Pb series of
boards.

Signed-off-by: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
---
 arch/mips/au1000/common/platform.c |   24 +++++
 drivers/i2c/busses/i2c-au1550.c    |  166 ++++++++++++++++++++++--------------
 drivers/i2c/busses/i2c-au1550.h    |   32 -------
 3 files changed, 126 insertions(+), 96 deletions(-)
 delete mode 100644 drivers/i2c/busses/i2c-au1550.h

diff --git a/arch/mips/au1000/common/platform.c b/arch/mips/au1000/common/platform.c
index d51e18f..5f47674 100644
--- a/arch/mips/au1000/common/platform.c
+++ b/arch/mips/au1000/common/platform.c
@@ -270,6 +270,27 @@ static struct platform_device smc91x_device = {
 
 #endif
 
+/* All Alchemy demoboards with I2C have this #define in their headers */
+#ifdef SMBUS_PSC_BASE
+static struct resource pbdb_smbus_resources[] = {
+	{
+		.start  = SMBUS_PSC_BASE,
+		.end    = SMBUS_PSC_BASE + 0x24 - 1,
+		.flags  = IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device pbdb_smbus_device = {
+	.dev = {
+		.platform_data = (void *)0,	/* bus number */
+	},
+	.name           = "au1xpsc_smbus",
+	.id             = 0,
+	.num_resources  = ARRAY_SIZE(pbdb_smbus_resources),
+	.resource       = pbdb_smbus_resources,
+};
+#endif
+
 static struct platform_device *au1xxx_platform_devices[] __initdata = {
 	&au1xxx_usb_ohci_device,
 	&au1x00_pcmcia_device,
@@ -287,6 +308,9 @@ static struct platform_device *au1xxx_platform_devices[] __initdata = {
 #ifdef CONFIG_MIPS_DB1200
 	&smc91x_device,
 #endif
+#ifdef SMBUS_PSC_BASE
+	&pbdb_smbus_device,
+#endif
 };
 
 int __init au1xxx_platform_init(void)
diff --git a/drivers/i2c/busses/i2c-au1550.c b/drivers/i2c/busses/i2c-au1550.c
index 7d51a43..a03c2d1 100644
--- a/drivers/i2c/busses/i2c-au1550.c
+++ b/drivers/i2c/busses/i2c-au1550.c
@@ -30,14 +30,22 @@
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/slab.h>
 
 #include <asm/mach-au1x00/au1xxx.h>
 #include <asm/mach-au1x00/au1xxx_psc.h>
 
-#include "i2c-au1550.h"
+struct i2c_au1550_data {
+	u32	psc_base;
+	int	xfer_timeout;
+	int	ack_timeout;
+	struct i2c_adapter adap;
+	struct resource *ioarea;
+};
 
 static int
 wait_xfer_done(struct i2c_au1550_data *adap)
@@ -299,18 +307,48 @@ static const struct i2c_algorithm au1550_algo = {
  * Prior to calling us, the 50MHz clock frequency and routing
  * must have been set up for the PSC indicated by the adapter.
  */
-int
-i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
+static int __devinit
+i2c_au1550_probe(struct platform_device *pdev)
 {
-	struct i2c_au1550_data *adap = i2c_adap->algo_data;
-	volatile psc_smb_t	*sp;
-	u32	stat;
+	struct i2c_au1550_data *priv;
+	volatile psc_smb_t *sp;
+	struct resource *r;
+	u32 stat;
+	int ret;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		ret = -ENODEV;
+		goto out;
+	}
 
-	i2c_adap->algo = &au1550_algo;
+	priv = kzalloc(sizeof(struct i2c_au1550_data), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	priv->ioarea = request_mem_region(r->start, r->end - r->start + 1,
+					  pdev->name);
+	if (!priv->ioarea) {
+		ret = -EBUSY;
+		goto out_mem;
+	}
+
+	priv->psc_base = r->start;
+	priv->xfer_timeout = 200;
+	priv->ack_timeout = 200;
+
+	priv->adap.id = I2C_HW_AU1550_PSC;
+	priv->adap.nr = (int)pdev->dev.platform_data;
+	priv->adap.algo = &au1550_algo;
+	priv->adap.algo_data = priv;
+	priv->adap.dev.parent = &pdev->dev;
+	strlcpy(priv->adap.name, "Au1xxx PSC I2C", 14);
 
 	/* Now, set up the PSC for SMBus PIO mode.
 	*/
-	sp = (volatile psc_smb_t *)(adap->psc_base);
+	sp = (volatile psc_smb_t *)priv->psc_base;
 	sp->psc_ctrl = PSC_CTRL_DISABLE;
 	au_sync();
 	sp->psc_sel = PSC_SEL_PS_SMBUSMODE;
@@ -348,87 +386,87 @@ i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
 		au_sync();
 	} while ((stat & PSC_SMBSTAT_DR) == 0);
 
-	return i2c_add_adapter(i2c_adap);
-}
+	ret = i2c_add_numbered_adapter(&priv->adap);
+	if (ret == 0) {
+		platform_set_drvdata(pdev, priv);
+		return 0;
+	}
+
+	/* disable the PSC */
+	sp->psc_smbcfg = 0;
+	sp->psc_ctrl = PSC_CTRL_DISABLE;
+	au_sync();
 
+	release_resource(priv->ioarea);
+	kfree(priv->ioarea);
+out_mem:
+	kfree(priv);
+out:
+	return ret;
+}
 
-int
-i2c_au1550_del_bus(struct i2c_adapter *adap)
+static int __devexit
+i2c_au1550_remove(struct platform_device *pdev)
 {
-	return i2c_del_adapter(adap);
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	volatile psc_smb_t *sp = (volatile psc_smb_t *)priv->psc_base;
+
+	platform_set_drvdata(pdev, NULL);
+	i2c_del_adapter(&priv->adap);
+	sp->psc_smbcfg = 0;
+	sp->psc_ctrl = PSC_CTRL_DISABLE;
+	au_sync();
+	release_resource(priv->ioarea);
+	kfree(priv->ioarea);
+	kfree(priv);
+	return 0;
 }
 
 static int
-pb1550_reg(struct i2c_client *client)
+i2c_au1550_suspend(struct platform_device *pdev, pm_message_t state)
 {
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	volatile psc_smb_t *sp = (volatile psc_smb_t *)priv->psc_base;
+
+	sp->psc_ctrl = PSC_CTRL_SUSPEND;
+	au_sync();
 	return 0;
 }
 
 static int
-pb1550_unreg(struct i2c_client *client)
+i2c_au1550_resume(struct platform_device *pdev)
 {
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	volatile psc_smb_t *sp = (volatile psc_smb_t *)priv->psc_base;
+
+	sp->psc_ctrl = PSC_CTRL_ENABLE;
+	au_sync();
+	while (!(sp->psc_smbstat & PSC_SMBSTAT_SR))
+		au_sync();
 	return 0;
 }
 
-static struct i2c_au1550_data pb1550_i2c_info = {
-	SMBUS_PSC_BASE, 200, 200
-};
-
-static struct i2c_adapter pb1550_board_adapter = {
-	name:              "pb1550 adapter",
-	id:                I2C_HW_AU1550_PSC,
-	algo:              NULL,
-	algo_data:         &pb1550_i2c_info,
-	client_register:   pb1550_reg,
-	client_unregister: pb1550_unreg,
+static struct platform_driver au1xpsc_smbus_driver = {
+	.driver = {
+		.name	= "au1xpsc_smbus",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= i2c_au1550_probe,
+	.remove		= __devexit_p(i2c_au1550_remove),
+	.suspend	= i2c_au1550_suspend,
+	.resume		= i2c_au1550_resume,
 };
 
-/* BIG hack to support the control interface on the Wolfson WM8731
- * audio codec on the Pb1550 board.  We get an address and two data
- * bytes to write, create an i2c message, and send it across the
- * i2c transfer function.  We do this here because we have access to
- * the i2c adapter structure.
- */
-static struct i2c_msg wm_i2c_msg;  /* We don't want this stuff on the stack */
-static	u8 i2cbuf[2];
-
-int
-pb1550_wm_codec_write(u8 addr, u8 reg, u8 val)
-{
-	wm_i2c_msg.addr = addr;
-	wm_i2c_msg.flags = 0;
-	wm_i2c_msg.buf = i2cbuf;
-	wm_i2c_msg.len = 2;
-	i2cbuf[0] = reg;
-	i2cbuf[1] = val;
-
-	return pb1550_board_adapter.algo->master_xfer(&pb1550_board_adapter, &wm_i2c_msg, 1);
-}
-
 static int __init
 i2c_au1550_init(void)
 {
-	printk(KERN_INFO "Au1550 I2C: ");
-
-	/* This is where we would set up a 50MHz clock source
-	 * and routing.  On the Pb1550, the SMBus is PSC2, which
-	 * uses a shared clock with USB.  This has been already
-	 * configured by Yamon as a 48MHz clock, close enough
-	 * for our work.
-	 */
-        if (i2c_au1550_add_bus(&pb1550_board_adapter) < 0) {
-		printk("failed to initialize.\n");
-                return -ENODEV;
-	}
-
-	printk("initialized.\n");
-	return 0;
+	return platform_driver_register(&au1xpsc_smbus_driver);
 }
 
 static void __exit
 i2c_au1550_exit(void)
 {
-	i2c_au1550_del_bus(&pb1550_board_adapter);
+	platform_driver_unregister(&au1xpsc_smbus_driver);
 }
 
 MODULE_AUTHOR("Dan Malek, Embedded Edge, LLC.");
diff --git a/drivers/i2c/busses/i2c-au1550.h b/drivers/i2c/busses/i2c-au1550.h
deleted file mode 100644
index fce15d1..0000000
--- a/drivers/i2c/busses/i2c-au1550.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Copyright (C) 2004 Embedded Edge, LLC <dan-L1vi/lXTdtu4JprhXtzr+Q@public.gmane.org>
- * 2.6 port by Matt Porter <mporter-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-
-#ifndef I2C_AU1550_H
-#define I2C_AU1550_H
-
-struct i2c_au1550_data {
-	u32	psc_base;
-	int	xfer_timeout;
-	int	ack_timeout;
-};
-
-int i2c_au1550_add_bus(struct i2c_adapter *);
-int i2c_au1550_del_bus(struct i2c_adapter *);
-
-#endif /* I2C_AU1550_H */
-- 
1.5.3.7


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-au1550: convert to platform driver

[Jean Delvare]

Hi Manuel,

On Fri, 18 Jan 2008 10:53:20 +0100, Manuel Lauss wrote:
> To make my humiliation complete, here's the third iteration
> of this patch. The original authors put a lot of 'volatile's
> in there for good reason; without them the driver hangs very
> early. The previous iteration removed them because of checkpatch
> complaining.
> (...)
> Convert the i2c-au1550 bus driver to platform driver, and
> register a platform device for the Alchemy Db/Pb series of
> boards.
> 
> Signed-off-by: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
> ---
>  arch/mips/au1000/common/platform.c |   24 +++++
>  drivers/i2c/busses/i2c-au1550.c    |  166 ++++++++++++++++++++++--------------
>  drivers/i2c/busses/i2c-au1550.h    |   32 -------
>  3 files changed, 126 insertions(+), 96 deletions(-)
>  delete mode 100644 drivers/i2c/busses/i2c-au1550.h

Thanks for the updated patch. Review:

> 
> diff --git a/arch/mips/au1000/common/platform.c b/arch/mips/au1000/common/platform.c
> index d51e18f..5f47674 100644
> --- a/arch/mips/au1000/common/platform.c
> +++ b/arch/mips/au1000/common/platform.c
> @@ -270,6 +270,27 @@ static struct platform_device smc91x_device = {
>  
>  #endif
>  
> +/* All Alchemy demoboards with I2C have this #define in their headers */
> +#ifdef SMBUS_PSC_BASE
> +static struct resource pbdb_smbus_resources[] = {
> +	{
> +		.start  = SMBUS_PSC_BASE,
> +		.end    = SMBUS_PSC_BASE + 0x24 - 1,
> +		.flags  = IORESOURCE_MEM,

You are supposed to achieve the alignment with tabs, not spaces. Your
original patch was correct, not sure why you changed it.

> +	},
> +};
> +
> +static struct platform_device pbdb_smbus_device = {
> +	.dev = {
> +		.platform_data = (void *)0,	/* bus number */
> +	},

I don't much like pointers abused to carry integers, and it doesn't
seem very helpful in this case: you can simply use .id below as the bus
number, as many other similar drivers do.

> +	.name           = "au1xpsc_smbus",
> +	.id             = 0,
> +	.num_resources  = ARRAY_SIZE(pbdb_smbus_resources),
> +	.resource       = pbdb_smbus_resources,

Here again the alignment should be achieved with tabs.

> +};
> +#endif
> +
>  static struct platform_device *au1xxx_platform_devices[] __initdata = {
>  	&au1xxx_usb_ohci_device,
>  	&au1x00_pcmcia_device,
> @@ -287,6 +308,9 @@ static struct platform_device *au1xxx_platform_devices[] __initdata = {
>  #ifdef CONFIG_MIPS_DB1200
>  	&smc91x_device,
>  #endif
> +#ifdef SMBUS_PSC_BASE
> +	&pbdb_smbus_device,
> +#endif
>  };
>  
>  int __init au1xxx_platform_init(void)
> diff --git a/drivers/i2c/busses/i2c-au1550.c b/drivers/i2c/busses/i2c-au1550.c
> index 7d51a43..a03c2d1 100644
> --- a/drivers/i2c/busses/i2c-au1550.c
> +++ b/drivers/i2c/busses/i2c-au1550.c
> @@ -30,14 +30,22 @@
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/platform_device.h>
>  #include <linux/init.h>
>  #include <linux/errno.h>
>  #include <linux/i2c.h>
> +#include <linux/slab.h>
>  
>  #include <asm/mach-au1x00/au1xxx.h>
>  #include <asm/mach-au1x00/au1xxx_psc.h>
>  
> -#include "i2c-au1550.h"
> +struct i2c_au1550_data {
> +	u32	psc_base;
> +	int	xfer_timeout;
> +	int	ack_timeout;
> +	struct i2c_adapter adap;
> +	struct resource *ioarea;
> +};
>  
>  static int
>  wait_xfer_done(struct i2c_au1550_data *adap)
> @@ -299,18 +307,48 @@ static const struct i2c_algorithm au1550_algo = {
>   * Prior to calling us, the 50MHz clock frequency and routing
>   * must have been set up for the PSC indicated by the adapter.
>   */
> -int
> -i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
> +static int __devinit
> +i2c_au1550_probe(struct platform_device *pdev)
>  {
> -	struct i2c_au1550_data *adap = i2c_adap->algo_data;
> -	volatile psc_smb_t	*sp;
> -	u32	stat;
> +	struct i2c_au1550_data *priv;
> +	volatile psc_smb_t *sp;
> +	struct resource *r;
> +	u32 stat;
> +	int ret;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!r) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
>  
> -	i2c_adap->algo = &au1550_algo;
> +	priv = kzalloc(sizeof(struct i2c_au1550_data), GFP_KERNEL);
> +	if (!priv) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	priv->ioarea = request_mem_region(r->start, r->end - r->start + 1,
> +					  pdev->name);
> +	if (!priv->ioarea) {
> +		ret = -EBUSY;
> +		goto out_mem;
> +	}
> +
> +	priv->psc_base = r->start;
> +	priv->xfer_timeout = 200;
> +	priv->ack_timeout = 200;
> +
> +	priv->adap.id = I2C_HW_AU1550_PSC;
> +	priv->adap.nr = (int)pdev->dev.platform_data;
> +	priv->adap.algo = &au1550_algo;
> +	priv->adap.algo_data = priv;
> +	priv->adap.dev.parent = &pdev->dev;
> +	strlcpy(priv->adap.name, "Au1xxx PSC I2C", 14);

The "size" argument is supposed to be the buffer size (i.e.
sizeof(priv->adap.name)), not the length of the string!

>  
>  	/* Now, set up the PSC for SMBus PIO mode.
>  	*/
> -	sp = (volatile psc_smb_t *)(adap->psc_base);
> +	sp = (volatile psc_smb_t *)priv->psc_base;
>  	sp->psc_ctrl = PSC_CTRL_DISABLE;
>  	au_sync();
>  	sp->psc_sel = PSC_SEL_PS_SMBUSMODE;
> @@ -348,87 +386,87 @@ i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
>  		au_sync();
>  	} while ((stat & PSC_SMBSTAT_DR) == 0);
>  
> -	return i2c_add_adapter(i2c_adap);
> -}
> +	ret = i2c_add_numbered_adapter(&priv->adap);
> +	if (ret == 0) {
> +		platform_set_drvdata(pdev, priv);
> +		return 0;
> +	}
> +
> +	/* disable the PSC */
> +	sp->psc_smbcfg = 0;
> +	sp->psc_ctrl = PSC_CTRL_DISABLE;
> +	au_sync();
>  
> +	release_resource(priv->ioarea);
> +	kfree(priv->ioarea);

I expected a call to release_mem_region() instead... but maybe that
works as well.

> +out_mem:
> +	kfree(priv);
> +out:
> +	return ret;
> +}
>  
> -int
> -i2c_au1550_del_bus(struct i2c_adapter *adap)
> +static int __devexit
> +i2c_au1550_remove(struct platform_device *pdev)
>  {
> -	return i2c_del_adapter(adap);
> +	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
> +	volatile psc_smb_t *sp = (volatile psc_smb_t *)priv->psc_base;
> +
> +	platform_set_drvdata(pdev, NULL);
> +	i2c_del_adapter(&priv->adap);
> +	sp->psc_smbcfg = 0;
> +	sp->psc_ctrl = PSC_CTRL_DISABLE;
> +	au_sync();
> +	release_resource(priv->ioarea);
> +	kfree(priv->ioarea);
> +	kfree(priv);
> +	return 0;
>  }
>  
>  static int
> -pb1550_reg(struct i2c_client *client)
> +i2c_au1550_suspend(struct platform_device *pdev, pm_message_t state)
>  {
> +	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
> +	volatile psc_smb_t *sp = (volatile psc_smb_t *)priv->psc_base;
> +
> +	sp->psc_ctrl = PSC_CTRL_SUSPEND;
> +	au_sync();
>  	return 0;
>  }
>  
>  static int
> -pb1550_unreg(struct i2c_client *client)
> +i2c_au1550_resume(struct platform_device *pdev)
>  {
> +	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
> +	volatile psc_smb_t *sp = (volatile psc_smb_t *)priv->psc_base;
> +
> +	sp->psc_ctrl = PSC_CTRL_ENABLE;
> +	au_sync();
> +	while (!(sp->psc_smbstat & PSC_SMBSTAT_SR))
> +		au_sync();
>  	return 0;
>  }
>  
> -static struct i2c_au1550_data pb1550_i2c_info = {
> -	SMBUS_PSC_BASE, 200, 200
> -};
> -
> -static struct i2c_adapter pb1550_board_adapter = {
> -	name:              "pb1550 adapter",
> -	id:                I2C_HW_AU1550_PSC,
> -	algo:              NULL,
> -	algo_data:         &pb1550_i2c_info,
> -	client_register:   pb1550_reg,
> -	client_unregister: pb1550_unreg,
> +static struct platform_driver au1xpsc_smbus_driver = {
> +	.driver = {
> +		.name	= "au1xpsc_smbus",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= i2c_au1550_probe,
> +	.remove		= __devexit_p(i2c_au1550_remove),
> +	.suspend	= i2c_au1550_suspend,
> +	.resume		= i2c_au1550_resume,
>  };
>  
> -/* BIG hack to support the control interface on the Wolfson WM8731
> - * audio codec on the Pb1550 board.  We get an address and two data
> - * bytes to write, create an i2c message, and send it across the
> - * i2c transfer function.  We do this here because we have access to
> - * the i2c adapter structure.
> - */
> -static struct i2c_msg wm_i2c_msg;  /* We don't want this stuff on the stack */
> -static	u8 i2cbuf[2];
> -
> -int
> -pb1550_wm_codec_write(u8 addr, u8 reg, u8 val)
> -{
> -	wm_i2c_msg.addr = addr;
> -	wm_i2c_msg.flags = 0;
> -	wm_i2c_msg.buf = i2cbuf;
> -	wm_i2c_msg.len = 2;
> -	i2cbuf[0] = reg;
> -	i2cbuf[1] = val;
> -
> -	return pb1550_board_adapter.algo->master_xfer(&pb1550_board_adapter, &wm_i2c_msg, 1);
> -}
> -
>  static int __init
>  i2c_au1550_init(void)
>  {
> -	printk(KERN_INFO "Au1550 I2C: ");
> -
> -	/* This is where we would set up a 50MHz clock source
> -	 * and routing.  On the Pb1550, the SMBus is PSC2, which
> -	 * uses a shared clock with USB.  This has been already
> -	 * configured by Yamon as a 48MHz clock, close enough
> -	 * for our work.
> -	 */
> -        if (i2c_au1550_add_bus(&pb1550_board_adapter) < 0) {
> -		printk("failed to initialize.\n");
> -                return -ENODEV;
> -	}
> -
> -	printk("initialized.\n");
> -	return 0;
> +	return platform_driver_register(&au1xpsc_smbus_driver);
>  }
>  
>  static void __exit
>  i2c_au1550_exit(void)
>  {
> -	i2c_au1550_del_bus(&pb1550_board_adapter);
> +	platform_driver_unregister(&au1xpsc_smbus_driver);
>  }
>  
>  MODULE_AUTHOR("Dan Malek, Embedded Edge, LLC.");
> diff --git a/drivers/i2c/busses/i2c-au1550.h b/drivers/i2c/busses/i2c-au1550.h
> deleted file mode 100644
> index fce15d1..0000000
> --- a/drivers/i2c/busses/i2c-au1550.h
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/*
> - * Copyright (C) 2004 Embedded Edge, LLC <dan-L1vi/lXTdtu4JprhXtzr+Q@public.gmane.org>
> - * 2.6 port by Matt Porter <mporter-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License
> - *  along with this program; if not, write to the Free Software
> - *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> - */
> -
> -#ifndef I2C_AU1550_H
> -#define I2C_AU1550_H
> -
> -struct i2c_au1550_data {
> -	u32	psc_base;
> -	int	xfer_timeout;
> -	int	ack_timeout;
> -};
> -
> -int i2c_au1550_add_bus(struct i2c_adapter *);
> -int i2c_au1550_del_bus(struct i2c_adapter *);
> -
> -#endif /* I2C_AU1550_H */

The rest looks just fine. Please address the few remaining comments,
retest and resubmit, and I'll add this patch to my i2c tree for 2.6.25.

Thanks for doing this BTW, it's nice to see this old driver finally
converted to fit in the device driver model :)

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

[PATCH] i2c-au1550: convert to platform driver #4

[Manuel Lauss]

Hi Jean,

Fixed all your concerns, except this one:

> > +	release_resource(priv->ioarea);
> > +	kfree(priv->ioarea);
> 
> I expected a call to release_mem_region() instead... but maybe that
> works as well.

Yes it does.  I like this one better because it only takes one parameter.

Thanks!
	Manuel Lauss

---

>From c81646c9d5861c606204b534ccc296439d36ec21 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
Date: Fri, 18 Jan 2008 10:03:17 +0100
Subject: [PATCH] i2c: convert i2c-au1550 to platform driver

Convert the i2c-au1550 bus driver to platform driver, and
register a platform device for the Alchemy Db/Pb series of
boards.

Signed-off-by: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
---
 arch/mips/au1000/common/platform.c |   21 +++++
 drivers/i2c/busses/i2c-au1550.c    |  166 ++++++++++++++++++++++--------------
 drivers/i2c/busses/i2c-au1550.h    |   32 -------
 3 files changed, 123 insertions(+), 96 deletions(-)
 delete mode 100644 drivers/i2c/busses/i2c-au1550.h

diff --git a/arch/mips/au1000/common/platform.c b/arch/mips/au1000/common/platform.c
index d51e18f..ec4bf3c 100644
--- a/arch/mips/au1000/common/platform.c
+++ b/arch/mips/au1000/common/platform.c
@@ -270,6 +270,24 @@ static struct platform_device smc91x_device = {
 
 #endif
 
+/* All Alchemy demoboards with I2C have this #define in their headers */
+#ifdef SMBUS_PSC_BASE
+static struct resource pbdb_smbus_resources[] = {
+	{
+		.start  = SMBUS_PSC_BASE,
+		.end    = SMBUS_PSC_BASE + 0x24 - 1,
+		.flags  = IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device pbdb_smbus_device = {
+	.name           = "au1xpsc_smbus",
+	.id             = 0,	/* bus number */
+	.num_resources  = ARRAY_SIZE(pbdb_smbus_resources),
+	.resource       = pbdb_smbus_resources,
+};
+#endif
+
 static struct platform_device *au1xxx_platform_devices[] __initdata = {
 	&au1xxx_usb_ohci_device,
 	&au1x00_pcmcia_device,
@@ -287,6 +305,9 @@ static struct platform_device *au1xxx_platform_devices[] __initdata = {
 #ifdef CONFIG_MIPS_DB1200
 	&smc91x_device,
 #endif
+#ifdef SMBUS_PSC_BASE
+	&pbdb_smbus_device,
+#endif
 };
 
 int __init au1xxx_platform_init(void)
diff --git a/drivers/i2c/busses/i2c-au1550.c b/drivers/i2c/busses/i2c-au1550.c
index 7d51a43..1953b26 100644
--- a/drivers/i2c/busses/i2c-au1550.c
+++ b/drivers/i2c/busses/i2c-au1550.c
@@ -30,14 +30,22 @@
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/slab.h>
 
 #include <asm/mach-au1x00/au1xxx.h>
 #include <asm/mach-au1x00/au1xxx_psc.h>
 
-#include "i2c-au1550.h"
+struct i2c_au1550_data {
+	u32	psc_base;
+	int	xfer_timeout;
+	int	ack_timeout;
+	struct i2c_adapter adap;
+	struct resource *ioarea;
+};
 
 static int
 wait_xfer_done(struct i2c_au1550_data *adap)
@@ -299,18 +307,48 @@ static const struct i2c_algorithm au1550_algo = {
  * Prior to calling us, the 50MHz clock frequency and routing
  * must have been set up for the PSC indicated by the adapter.
  */
-int
-i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
+static int __devinit
+i2c_au1550_probe(struct platform_device *pdev)
 {
-	struct i2c_au1550_data *adap = i2c_adap->algo_data;
-	volatile psc_smb_t	*sp;
-	u32	stat;
+	struct i2c_au1550_data *priv;
+	volatile psc_smb_t *sp;
+	struct resource *r;
+	u32 stat;
+	int ret;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		ret = -ENODEV;
+		goto out;
+	}
 
-	i2c_adap->algo = &au1550_algo;
+	priv = kzalloc(sizeof(struct i2c_au1550_data), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	priv->ioarea = request_mem_region(r->start, r->end - r->start + 1,
+					  pdev->name);
+	if (!priv->ioarea) {
+		ret = -EBUSY;
+		goto out_mem;
+	}
+
+	priv->psc_base = r->start;
+	priv->xfer_timeout = 200;
+	priv->ack_timeout = 200;
+
+	priv->adap.id = I2C_HW_AU1550_PSC;
+	priv->adap.nr = pdev->id;
+	priv->adap.algo = &au1550_algo;
+	priv->adap.algo_data = priv;
+	priv->adap.dev.parent = &pdev->dev;
+	strlcpy(priv->adap.name, "Au1xxx PSC I2C", sizeof(priv->adap.name));
 
 	/* Now, set up the PSC for SMBus PIO mode.
 	*/
-	sp = (volatile psc_smb_t *)(adap->psc_base);
+	sp = (volatile psc_smb_t *)priv->psc_base;
 	sp->psc_ctrl = PSC_CTRL_DISABLE;
 	au_sync();
 	sp->psc_sel = PSC_SEL_PS_SMBUSMODE;
@@ -348,87 +386,87 @@ i2c_au1550_add_bus(struct i2c_adapter *i2c_adap)
 		au_sync();
 	} while ((stat & PSC_SMBSTAT_DR) == 0);
 
-	return i2c_add_adapter(i2c_adap);
-}
+	ret = i2c_add_numbered_adapter(&priv->adap);
+	if (ret == 0) {
+		platform_set_drvdata(pdev, priv);
+		return 0;
+	}
+
+	/* disable the PSC */
+	sp->psc_smbcfg = 0;
+	sp->psc_ctrl = PSC_CTRL_DISABLE;
+	au_sync();
 
+	release_resource(priv->ioarea);
+	kfree(priv->ioarea);
+out_mem:
+	kfree(priv);
+out:
+	return ret;
+}
 
-int
-i2c_au1550_del_bus(struct i2c_adapter *adap)
+static int __devexit
+i2c_au1550_remove(struct platform_device *pdev)
 {
-	return i2c_del_adapter(adap);
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	volatile psc_smb_t *sp = (volatile psc_smb_t *)priv->psc_base;
+
+	platform_set_drvdata(pdev, NULL);
+	i2c_del_adapter(&priv->adap);
+	sp->psc_smbcfg = 0;
+	sp->psc_ctrl = PSC_CTRL_DISABLE;
+	au_sync();
+	release_resource(priv->ioarea);
+	kfree(priv->ioarea);
+	kfree(priv);
+	return 0;
 }
 
 static int
-pb1550_reg(struct i2c_client *client)
+i2c_au1550_suspend(struct platform_device *pdev, pm_message_t state)
 {
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	volatile psc_smb_t *sp = (volatile psc_smb_t *)priv->psc_base;
+
+	sp->psc_ctrl = PSC_CTRL_SUSPEND;
+	au_sync();
 	return 0;
 }
 
 static int
-pb1550_unreg(struct i2c_client *client)
+i2c_au1550_resume(struct platform_device *pdev)
 {
+	struct i2c_au1550_data *priv = platform_get_drvdata(pdev);
+	volatile psc_smb_t *sp = (volatile psc_smb_t *)priv->psc_base;
+
+	sp->psc_ctrl = PSC_CTRL_ENABLE;
+	au_sync();
+	while (!(sp->psc_smbstat & PSC_SMBSTAT_SR))
+		au_sync();
 	return 0;
 }
 
-static struct i2c_au1550_data pb1550_i2c_info = {
-	SMBUS_PSC_BASE, 200, 200
-};
-
-static struct i2c_adapter pb1550_board_adapter = {
-	name:              "pb1550 adapter",
-	id:                I2C_HW_AU1550_PSC,
-	algo:              NULL,
-	algo_data:         &pb1550_i2c_info,
-	client_register:   pb1550_reg,
-	client_unregister: pb1550_unreg,
+static struct platform_driver au1xpsc_smbus_driver = {
+	.driver = {
+		.name	= "au1xpsc_smbus",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= i2c_au1550_probe,
+	.remove		= __devexit_p(i2c_au1550_remove),
+	.suspend	= i2c_au1550_suspend,
+	.resume		= i2c_au1550_resume,
 };
 
-/* BIG hack to support the control interface on the Wolfson WM8731
- * audio codec on the Pb1550 board.  We get an address and two data
- * bytes to write, create an i2c message, and send it across the
- * i2c transfer function.  We do this here because we have access to
- * the i2c adapter structure.
- */
-static struct i2c_msg wm_i2c_msg;  /* We don't want this stuff on the stack */
-static	u8 i2cbuf[2];
-
-int
-pb1550_wm_codec_write(u8 addr, u8 reg, u8 val)
-{
-	wm_i2c_msg.addr = addr;
-	wm_i2c_msg.flags = 0;
-	wm_i2c_msg.buf = i2cbuf;
-	wm_i2c_msg.len = 2;
-	i2cbuf[0] = reg;
-	i2cbuf[1] = val;
-
-	return pb1550_board_adapter.algo->master_xfer(&pb1550_board_adapter, &wm_i2c_msg, 1);
-}
-
 static int __init
 i2c_au1550_init(void)
 {
-	printk(KERN_INFO "Au1550 I2C: ");
-
-	/* This is where we would set up a 50MHz clock source
-	 * and routing.  On the Pb1550, the SMBus is PSC2, which
-	 * uses a shared clock with USB.  This has been already
-	 * configured by Yamon as a 48MHz clock, close enough
-	 * for our work.
-	 */
-        if (i2c_au1550_add_bus(&pb1550_board_adapter) < 0) {
-		printk("failed to initialize.\n");
-                return -ENODEV;
-	}
-
-	printk("initialized.\n");
-	return 0;
+	return platform_driver_register(&au1xpsc_smbus_driver);
 }
 
 static void __exit
 i2c_au1550_exit(void)
 {
-	i2c_au1550_del_bus(&pb1550_board_adapter);
+	platform_driver_unregister(&au1xpsc_smbus_driver);
 }
 
 MODULE_AUTHOR("Dan Malek, Embedded Edge, LLC.");
diff --git a/drivers/i2c/busses/i2c-au1550.h b/drivers/i2c/busses/i2c-au1550.h
deleted file mode 100644
index fce15d1..0000000
--- a/drivers/i2c/busses/i2c-au1550.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Copyright (C) 2004 Embedded Edge, LLC <dan-L1vi/lXTdtu4JprhXtzr+Q@public.gmane.org>
- * 2.6 port by Matt Porter <mporter-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-
-#ifndef I2C_AU1550_H
-#define I2C_AU1550_H
-
-struct i2c_au1550_data {
-	u32	psc_base;
-	int	xfer_timeout;
-	int	ack_timeout;
-};
-
-int i2c_au1550_add_bus(struct i2c_adapter *);
-int i2c_au1550_del_bus(struct i2c_adapter *);
-
-#endif /* I2C_AU1550_H */
-- 
1.5.3.8


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH] i2c-au1550: convert to platform driver #4

[Jean Delvare]

Hi Manuel,

On Mon, 21 Jan 2008 09:47:06 +0100, Manuel Lauss wrote:
> Hi Jean,
> 
> Fixed all your concerns, except this one:
> 
> > > +	release_resource(priv->ioarea);
> > > +	kfree(priv->ioarea);
> > 
> > I expected a call to release_mem_region() instead... but maybe that
> > works as well.
> 
> Yes it does.  I like this one better because it only takes one parameter.

OK, that's fine with me.

> (...)
> Convert the i2c-au1550 bus driver to platform driver, and
> register a platform device for the Alchemy Db/Pb series of
> boards.
> 
> Signed-off-by: Manuel Lauss <mano-nEyxjcs6f3Vin2gBucwGBecsttgLyre6@public.gmane.org>
> ---
>  arch/mips/au1000/common/platform.c |   21 +++++
>  drivers/i2c/busses/i2c-au1550.c    |  166 ++++++++++++++++++++++--------------
>  drivers/i2c/busses/i2c-au1550.h    |   32 -------
>  3 files changed, 123 insertions(+), 96 deletions(-)
>  delete mode 100644 drivers/i2c/busses/i2c-au1550.h

Patch applied, thanks!

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c