On Fri, Apr 20, 2007 at 08:27:14AM +0400, Vitaly Bordug wrote:

  

diff --git a/arch/powerpc/platforms/8xx/mpc885ads_setup.c b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
index 9bd81c7..d32e066 100644
--- a/arch/powerpc/platforms/8xx/mpc885ads_setup.c
+++ b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
@@ -51,6 +51,7 @@ static void init_smc1_uart_ioports(struc
 static void init_smc2_uart_ioports(struct fs_uart_platform_info* fpi);
 static void init_scc3_ioports(struct fs_platform_info* ptr);
 static void init_irda_ioports(void);
+static void init_i2c_ioports(void);
 
 void __init mpc885ads_board_setup(void)
 {
@@ -120,6 +121,10 @@ #endif
 #ifdef CONFIG_8XX_SIR
 	init_irda_ioports();
 #endif
+
+#ifdef CONFIG_I2C_RPXLITE
+	init_i2c_ioports();
+#endif
    

Does it hurt to always do it, even when the driver is not enabled? THat'd
do away with an ifdef.

  

Also, if you move the static function up, you don't need a prototype. That
goes for other stuff in this file too.

  

 }
 
 
@@ -361,6 +366,15 @@ static void init_irda_ioports()
 	immr_unmap(cp);
 }
 
+static void init_i2c_ioports()
+{
+	cpm8xx_t *cp = (cpm8xx_t *)immr_map(im_cpm);
+
+        setbits32(&cp->cp_pbpar, 0x00000030);
+        setbits32(&cp->cp_pbdir, 0x00000030);
+        setbits16(&cp->cp_pbodr, 0x0030);
+}
    

Looks like you moved this out of the driver and into the platform
code. What happens to other platforms where it's used?

  

+
 int platform_device_skip(const char *model, int id)
 {
 #ifdef CONFIG_MPC8xx_SECOND_ETH_SCC3
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 419b688..7ecd537 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -331,7 +331,7 @@ static int __init fsl_i2c_of_init(void)
 	for (np = NULL, i = 0;
 	     (np = of_find_compatible_node(np, "i2c", "fsl-i2c")) != NULL;
 	     i++) {
-		struct resource r[2];
+		struct resource r[3];
    

Why? No code that uses it has been changed. Is it a bugfix?

  

 		struct fsl_i2c_platform_data i2c_data;
 		const unsigned char *flags = NULL;
 
@@ -1215,4 +1215,63 @@ err:
 
 arch_initcall(fs_irda_of_init);
 
+static const char *i2c_regs = "regs";
+static const char *i2c_pram = "pram";
+static const char *i2c_irq = "interrupt";
+
+static int __init fsl_i2c_cpm_of_init(void)
+{
+	struct device_node *np;
+	unsigned int i;
+	struct platform_device *i2c_dev;
+	int ret;
+
+	for (np = NULL, i = 0;
+	     (np = of_find_compatible_node(np, "i2c", "fsl-i2c-cpm")) != NULL;
+	     i++) {
+		struct resource r[3];
+		struct fsl_i2c_platform_data i2c_data;
+
+		memset(&r, 0, sizeof(r));
+		memset(&i2c_data, 0, sizeof(i2c_data));
+
+		ret = of_address_to_resource(np, 0, &r[0]);
+		if (ret)
+			goto err;
+		r[0].name = i2c_regs;
+
+		ret = of_address_to_resource(np, 1, &r[1]);
+		if (ret)
+			goto err;
+		r[1].name = i2c_pram;
+
+		r[2].start = r[2].end = irq_of_parse_and_map(np, 0);
+		r[2].flags = IORESOURCE_IRQ;
+		r[2].name = i2c_irq;
+
+		i2c_dev = platform_device_register_simple("fsl-i2c-cpm", i, &r[0], 3);
+		if (IS_ERR(i2c_dev)) {
+			ret = PTR_ERR(i2c_dev);
+			goto err;
+		}
+
+		ret =
+		    platform_device_add_data(i2c_dev, &i2c_data,
+					     sizeof(struct
+						    fsl_i2c_platform_data));
+		if (ret)
+			goto unreg;
+	}
+
+	return 0;
+
+unreg:
+	platform_device_unregister(i2c_dev);
+err:
+	return ret;
+}
+
+arch_initcall(fsl_i2c_cpm_of_init);
    

This could all be done with an of_platform driver instead, and avoid the above.
(Someone else already suggested that I believe).