[PATCH v2 0/3] i2c-mpc: add support for the Freescale MPC512x and other fixes

[PATCH v2 0/3] i2c-mpc: add support for the Freescale MPC512x and other fixes

[Wolfgang Grandegger]

From: Wolfgang Grandegger <wg@denx.de>

This patch series adds support for the MPC512x from Freescale to the
i2c-mpc driver. At that occasion, issues with  __devinit[data] have
been fixed and the doc of the FSL I2C dts bindings updated. It has
been tested on a MPC5121ADS, TQM5200 and TQM8560 board

Changes since v1:

- use macro MPC_I2C_CLOCK_PRESERVE/SAFE for the special clock settings.
- document the special DTS node "fsl,mpc5121-i2c-ctrl".
- update and correct the Kconfig help.
- some other minor fixes as suggested by Wolfram.

Wolfgang

Wolfgang Grandegger (3):
  i2c-mpc: use __devinit[data] for initialization functions and data
  i2c-mpc: add support for the MPC512x processors from Freescale
  powerpc: doc/dts-bindings: update doc of FSL I2C bindings

 Documentation/powerpc/dts-bindings/fsl/i2c.txt |   30 +++-
 drivers/i2c/busses/Kconfig                     |    7 +-
 drivers/i2c/busses/i2c-mpc.c                   |  190 +++++++++++++++---------
 3 files changed, 147 insertions(+), 80 deletions(-)

[PATCH v2 1/3] i2c-mpc: use __devinit[data] for initialization functions and data

[Wolfgang Grandegger]

From: Wolfgang Grandegger <wg@denx.de>

"__devinit[data]" has not yet been used for all initialization functions
and data. To avoid truncating lines, the struct mpc_i2c_match_data has
been renamed to mpc_i2c_data, which is even the better name.

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
---
 drivers/i2c/busses/i2c-mpc.c |   99 +++++++++++++++++++----------------------
 1 files changed, 46 insertions(+), 53 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index f627001..2cb864e 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -66,7 +66,7 @@ struct mpc_i2c_divider {
 	u16 fdr;	/* including dfsrr */
 };
 
-struct mpc_i2c_match_data {
+struct mpc_i2c_data {
 	void (*setclock)(struct device_node *node,
 			 struct mpc_i2c *i2c,
 			 u32 clock, u32 prescaler);
@@ -165,7 +165,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
 }
 
 #ifdef CONFIG_PPC_MPC52xx
-static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
+static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
 	{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
 	{28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
 	{36, 0x26}, {40, 0x27}, {44, 0x04}, {48, 0x28},
@@ -186,7 +186,8 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
 	{10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f}
 };
 
-int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
+static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
+					  int prescaler)
 {
 	const struct mpc_i2c_divider *div = NULL;
 	unsigned int pvr = mfspr(SPRN_PVR);
@@ -215,9 +216,9 @@ int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
 	return div ? (int)div->fdr : -EINVAL;
 }
 
-static void mpc_i2c_setclock_52xx(struct device_node *node,
-				  struct mpc_i2c *i2c,
-				  u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
+					    struct mpc_i2c *i2c,
+					    u32 clock, u32 prescaler)
 {
 	int ret, fdr;
 
@@ -230,15 +231,15 @@ static void mpc_i2c_setclock_52xx(struct device_node *node,
 		dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
 }
 #else /* !CONFIG_PPC_MPC52xx */
-static void mpc_i2c_setclock_52xx(struct device_node *node,
-				  struct mpc_i2c *i2c,
-				  u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
+					    struct mpc_i2c *i2c,
+					    u32 clock, u32 prescaler)
 {
 }
 #endif /* CONFIG_PPC_MPC52xx*/
 
 #ifdef CONFIG_FSL_SOC
-static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
+static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
 	{160, 0x0120}, {192, 0x0121}, {224, 0x0122}, {256, 0x0123},
 	{288, 0x0100}, {320, 0x0101}, {352, 0x0601}, {384, 0x0102},
 	{416, 0x0602}, {448, 0x0126}, {480, 0x0103}, {512, 0x0127},
@@ -258,7 +259,7 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
 	{49152, 0x011e}, {61440, 0x011f}
 };
 
-u32 mpc_i2c_get_sec_cfg_8xxx(void)
+static u32 __devinit mpc_i2c_get_sec_cfg_8xxx(void)
 {
 	struct device_node *node = NULL;
 	u32 __iomem *reg;
@@ -287,7 +288,8 @@ u32 mpc_i2c_get_sec_cfg_8xxx(void)
 	return val;
 }
 
-int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
+static int __devinit mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
+					  u32 prescaler)
 {
 	const struct mpc_i2c_divider *div = NULL;
 	u32 divider;
@@ -320,9 +322,9 @@ int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
 	return div ? (int)div->fdr : -EINVAL;
 }
 
-static void mpc_i2c_setclock_8xxx(struct device_node *node,
-				  struct mpc_i2c *i2c,
-				  u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
+					    struct mpc_i2c *i2c,
+					    u32 clock, u32 prescaler)
 {
 	int ret, fdr;
 
@@ -338,9 +340,9 @@ static void mpc_i2c_setclock_8xxx(struct device_node *node,
 }
 
 #else /* !CONFIG_FSL_SOC */
-static void mpc_i2c_setclock_8xxx(struct device_node *node,
-				  struct mpc_i2c *i2c,
-				  u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
+					    struct mpc_i2c *i2c,
+					    u32 clock, u32 prescaler)
 {
 }
 #endif /* CONFIG_FSL_SOC */
@@ -529,8 +531,8 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
 			clock = *prop;
 
 		if (match->data) {
-			struct mpc_i2c_match_data *data =
-				(struct mpc_i2c_match_data *)match->data;
+			struct mpc_i2c_data *data =
+				(struct mpc_i2c_data *)match->data;
 			data->setclock(op->node, i2c, clock, data->prescaler);
 		} else {
 			/* Backwards compatibility */
@@ -582,44 +584,35 @@ static int __devexit fsl_i2c_remove(struct of_device *op)
 	return 0;
 };
 
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_52xx = {
+	.setclock = mpc_i2c_setclock_52xx,
+};
+
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_8313 = {
+	.setclock = mpc_i2c_setclock_8xxx,
+};
+
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_8543 = {
+	.setclock = mpc_i2c_setclock_8xxx,
+	.prescaler = 2,
+};
+
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_8544 = {
+	.setclock = mpc_i2c_setclock_8xxx,
+	.prescaler = 3,
+};
+
 static const struct of_device_id mpc_i2c_of_match[] = {
-	{.compatible = "mpc5200-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_52xx,
-		},
-	},
-	{.compatible = "fsl,mpc5200b-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_52xx,
-		},
-	},
-	{.compatible = "fsl,mpc5200-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_52xx,
-		},
-	},
-	{.compatible = "fsl,mpc8313-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_8xxx,
-		},
-	},
-	{.compatible = "fsl,mpc8543-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_8xxx,
-			.prescaler = 2,
-		},
-	},
-	{.compatible = "fsl,mpc8544-i2c",
-	 .data = &(struct mpc_i2c_match_data) {
-			.setclock = mpc_i2c_setclock_8xxx,
-			.prescaler = 3,
-		},
+	{.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
+	{.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
+	{.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
+	{.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
+	{.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
+	{.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
 	/* Backward compatibility */
-	},
 	{.compatible = "fsl-i2c", },
 	{},
 };
-
 MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
 
 
-- 
1.6.2.5

[PATCH v2 2/3] i2c-mpc: add support for the MPC512x processors from Freescale

[Wolfgang Grandegger]

From: Wolfgang Grandegger <wg@denx.de>

The "setclock" initialization functions have been renamed to "setup"
because I2C interrupts must be enabled for the MPC512x. This requires
to handle "fsl,preserve-clocking" in a slighly different way. Also,
the old settings are now reported calling dev_dbg(). For the MPC512x
the clock setup function of the MPC52xx can be re-used. Furthermore,
the Kconfig help has been updated and corrected.

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
---
 drivers/i2c/busses/Kconfig   |    7 +-
 drivers/i2c/busses/i2c-mpc.c |  127 ++++++++++++++++++++++++++++++------------
 2 files changed, 95 insertions(+), 39 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 5f318ce..5477e41 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -418,13 +418,12 @@ config I2C_IXP2000
 	  instead.
 
 config I2C_MPC
-	tristate "MPC107/824x/85xx/52xx/86xx"
+	tristate "MPC107/824x/85xx/512x/52xx/83xx/86xx"
 	depends on PPC32
 	help
 	  If you say yes to this option, support will be included for the
-	  built-in I2C interface on the MPC107/Tsi107/MPC8240/MPC8245 and
-	  MPC85xx/MPC8641 family processors. The driver may also work on 52xx
-	  family processors, though interrupts are known not to work.
+	  built-in I2C interface on the MPC107, Tsi107, MPC512x, MPC52xx,
+	  MPC8240, MPC8245, MPC83xx, MPC85xx and MPC8641 family processors.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mpc.
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 2cb864e..bc0281d 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -31,6 +31,9 @@
 
 #define DRV_NAME "mpc-i2c"
 
+#define MPC_I2C_CLOCK_SAFE     0
+#define MPC_I2C_CLOCK_PRESERVE (~0U)
+
 #define MPC_I2C_FDR   0x04
 #define MPC_I2C_CR    0x08
 #define MPC_I2C_SR    0x0c
@@ -67,9 +70,8 @@ struct mpc_i2c_divider {
 };
 
 struct mpc_i2c_data {
-	void (*setclock)(struct device_node *node,
-			 struct mpc_i2c *i2c,
-			 u32 clock, u32 prescaler);
+	void (*setup)(struct device_node *node, struct mpc_i2c *i2c,
+		      u32 clock, u32 prescaler);
 	u32 prescaler;
 };
 
@@ -164,7 +166,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
 	return 0;
 }
 
-#ifdef CONFIG_PPC_MPC52xx
+#if defined(CONFIG_PPC_MPC52xx) || defined(CONFIG_PPC_MPC512x)
 static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
 	{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
 	{28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
@@ -216,12 +218,18 @@ static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
 	return div ? (int)div->fdr : -EINVAL;
 }
 
-static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
-					    struct mpc_i2c *i2c,
-					    u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setup_52xx(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
 {
 	int ret, fdr;
 
+	if (clock == MPC_I2C_CLOCK_PRESERVE) {
+		dev_dbg(i2c->dev, "using fdr %d\n",
+			readb(i2c->base + MPC_I2C_FDR));
+		return;
+	}
+
 	ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler);
 	fdr = (ret >= 0) ? ret : 0x3f; /* backward compatibility */
 
@@ -230,13 +238,50 @@ static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
 	if (ret >= 0)
 		dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
 }
-#else /* !CONFIG_PPC_MPC52xx */
-static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
-					    struct mpc_i2c *i2c,
-					    u32 clock, u32 prescaler)
+#else /* !(CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x) */
+static void __devinit mpc_i2c_setup_52xx(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
 {
 }
-#endif /* CONFIG_PPC_MPC52xx*/
+#endif /* CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x */
+
+#ifdef CONFIG_PPC_MPC512x
+static void __devinit mpc_i2c_setup_512x(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
+{
+	struct device_node *node_ctrl;
+	void __iomem *ctrl;
+	const u32 *pval;
+	u32 idx;
+
+	/* Enable I2C interrupts for mpc5121 */
+	node_ctrl = of_find_compatible_node(NULL, NULL,
+					    "fsl,mpc5121-i2c-ctrl");
+	if (node_ctrl) {
+		ctrl = of_iomap(node_ctrl, 0);
+		if (ctrl) {
+
+			/* Interrupt enable bits for i2c-0/1/2: bit 24/26/28 */
+			pval = of_get_property(node, "reg", NULL);
+			idx = (*pval & 0xff) / 0x20;
+			setbits32(ctrl, 1 << (24 + idx * 2));
+			iounmap(ctrl);
+		}
+		of_node_put(node_ctrl);
+	}
+
+	/* The clock setup for the 52xx works also fine for the 512x */
+	mpc_i2c_setup_52xx(node, i2c, clock, prescaler);
+}
+#else /* CONFIG_PPC_MPC512x */
+static void __devinit mpc_i2c_setup_512x(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
+{
+}
+#endif /* CONFIG_PPC_MPC512x */
 
 #ifdef CONFIG_FSL_SOC
 static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
@@ -322,12 +367,19 @@ static int __devinit mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
 	return div ? (int)div->fdr : -EINVAL;
 }
 
-static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
-					    struct mpc_i2c *i2c,
-					    u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setup_8xxx(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
 {
 	int ret, fdr;
 
+	if (clock == MPC_I2C_CLOCK_PRESERVE) {
+		dev_dbg(i2c->dev, "using dfsrr %d, fdr %d\n",
+			readb(i2c->base + MPC_I2C_DFSRR),
+			readb(i2c->base + MPC_I2C_FDR));
+		return;
+	}
+
 	ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler);
 	fdr = (ret >= 0) ? ret : 0x1031; /* backward compatibility */
 
@@ -340,9 +392,9 @@ static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
 }
 
 #else /* !CONFIG_FSL_SOC */
-static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
-					    struct mpc_i2c *i2c,
-					    u32 clock, u32 prescaler)
+static void __devinit mpc_i2c_setup_8xxx(struct device_node *node,
+					 struct mpc_i2c *i2c,
+					 u32 clock, u32 prescaler)
 {
 }
 #endif /* CONFIG_FSL_SOC */
@@ -496,7 +548,7 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
 {
 	struct mpc_i2c *i2c;
 	const u32 *prop;
-	u32 clock = 0;
+	u32 clock = MPC_I2C_CLOCK_SAFE;
 	int result = 0;
 	int plen;
 
@@ -525,21 +577,21 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
 		}
 	}
 
-	if (!of_get_property(op->node, "fsl,preserve-clocking", NULL)) {
+	if (of_get_property(op->node, "fsl,preserve-clocking", NULL)) {
+		clock = MPC_I2C_PRESERVE_CLOCK;
+	} else {
 		prop = of_get_property(op->node, "clock-frequency", &plen);
 		if (prop && plen == sizeof(u32))
 			clock = *prop;
+	}
 
-		if (match->data) {
-			struct mpc_i2c_data *data =
-				(struct mpc_i2c_data *)match->data;
-			data->setclock(op->node, i2c, clock, data->prescaler);
-		} else {
-			/* Backwards compatibility */
-			if (of_get_property(op->node, "dfsrr", NULL))
-				mpc_i2c_setclock_8xxx(op->node, i2c,
-						      clock, 0);
-		}
+	if (match->data) {
+		struct mpc_i2c_data *data = match->data;
+		data->setup(op->node, i2c, clock, data->prescaler);
+	} else {
+		/* Backwards compatibility */
+		if (of_get_property(op->node, "dfsrr", NULL))
+			mpc_i2c_setup_8xxx(op->node, i2c, clock, 0);
 	}
 
 	dev_set_drvdata(&op->dev, i2c);
@@ -584,21 +636,25 @@ static int __devexit fsl_i2c_remove(struct of_device *op)
 	return 0;
 };
 
+static struct mpc_i2c_data __devinitdata mpc_i2c_data_512x = {
+	.setup = mpc_i2c_setup_512x,
+};
+
 static struct mpc_i2c_data __devinitdata mpc_i2c_data_52xx = {
-	.setclock = mpc_i2c_setclock_52xx,
+	.setup = mpc_i2c_setup_52xx,
 };
 
 static struct mpc_i2c_data __devinitdata mpc_i2c_data_8313 = {
-	.setclock = mpc_i2c_setclock_8xxx,
+	.setup = mpc_i2c_setup_8xxx,
 };
 
 static struct mpc_i2c_data __devinitdata mpc_i2c_data_8543 = {
-	.setclock = mpc_i2c_setclock_8xxx,
+	.setup = mpc_i2c_setup_8xxx,
 	.prescaler = 2,
 };
 
 static struct mpc_i2c_data __devinitdata mpc_i2c_data_8544 = {
-	.setclock = mpc_i2c_setclock_8xxx,
+	.setup = mpc_i2c_setup_8xxx,
 	.prescaler = 3,
 };
 
@@ -606,6 +662,7 @@ static const struct of_device_id mpc_i2c_of_match[] = {
 	{.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
 	{.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
 	{.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
+	{.compatible = "fsl,mpc5121-i2c", .data = &mpc_i2c_data_512x, },
 	{.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
 	{.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
 	{.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
@@ -648,5 +705,5 @@ module_exit(fsl_i2c_exit);
 
 MODULE_AUTHOR("Adrian Cox <adrian@humboldt.co.uk>");
 MODULE_DESCRIPTION("I2C-Bus adapter for MPC107 bridge and "
-		   "MPC824x/85xx/52xx processors");
+		   "MPC824x/85xx/512x/52xx processors");
 MODULE_LICENSE("GPL");
-- 
1.6.2.5

[PATCH v2 3/3] powerpc: doc/dts-bindings: update doc of FSL I2C bindings

[Wolfgang Grandegger]

From: Wolfgang Grandegger <wg@denx.de>

This patch adds the MPC5121 to the list of supported devices,
enhances the doc of the "clock-frequency" property and removes
the obsolete "cell-index" property from the example nodes.
Furthermore and example for the MPC5121 has been added.

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
---
 Documentation/powerpc/dts-bindings/fsl/i2c.txt |   30 +++++++++++++++++++----
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/Documentation/powerpc/dts-bindings/fsl/i2c.txt b/Documentation/powerpc/dts-bindings/fsl/i2c.txt
index b6d2e21..2f62dae 100644
--- a/Documentation/powerpc/dts-bindings/fsl/i2c.txt
+++ b/Documentation/powerpc/dts-bindings/fsl/i2c.txt
@@ -9,8 +9,9 @@ Recommended properties :
 
  - compatible : compatibility list with 2 entries, the first should
    be "fsl,CHIP-i2c" where CHIP is the name of a compatible processor,
-   e.g. mpc8313, mpc8543, mpc8544, mpc5200 or mpc5200b. The second one
-   should be "fsl-i2c".
+   e.g. mpc8313, mpc8543, mpc8544, mpc5121, mpc5200 or mpc5200b. The
+   second one should be "fsl-i2c". For the mpc5121, an additional node
+   "fsl,mpc5121-i2c-ctrl" is required as shown in the example below.
  - interrupts : <a b> where a is the interrupt number and b is a
    field that represents an encoding of the sense and level
    information for the interrupt.  This should be encoded based on
@@ -20,29 +21,46 @@ Recommended properties :
    services interrupts for this device.
  - fsl,preserve-clocking : boolean; if defined, the clock settings
    from the bootloader are preserved (not touched).
- - clock-frequency : desired I2C bus clock frequency in Hz.
+ - clock-frequency : desired I2C bus clock frequency in Hz.  If this
+   property and "fsl,preserve-clocking" is not defined, a safe fixed
+   clock divider value is used (resulting in a small clock frequency).
 
 Examples :
 
+	/* MPC5121 based board */
+	i2c@1740 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "fsl,mpc5121-i2c", "fsl-i2c";
+		reg = <0x1740 0x20>;
+		interrupts = <11 0x8>;
+		interrupt-parent = <&ipic>;
+		clock-frequency = <100000>;
+	};
+
+	i2ccontrol@1760 {
+		compatible = "fsl,mpc5121-i2c-ctrl";
+		reg = <0x1760 0x8>;
+	};
+
+	/* MPC5200B based board */
 	i2c@3d00 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
-		cell-index = <0>;
 		reg = <0x3d00 0x40>;
 		interrupts = <2 15 0>;
 		interrupt-parent = <&mpc5200_pic>;
 		fsl,preserve-clocking;
 	};
 
+	/* MPC8544 base board */
 	i2c@3100 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		cell-index = <1>;
 		compatible = "fsl,mpc8544-i2c", "fsl-i2c";
 		reg = <0x3100 0x100>;
 		interrupts = <43 2>;
 		interrupt-parent = <&mpic>;
 		clock-frequency = <400000>;
 	};
-
-- 
1.6.2.5

Re: [PATCH v2 1/3] i2c-mpc: use __devinit[data] for initialization functions and data

[Ben Dooks]

On Mon, Jan 25, 2010 at 09:55:04PM +0100, Wolfgang Grandegger wrote:
> From: Wolfgang Grandegger <wg@denx.de>
> 
> "__devinit[data]" has not yet been used for all initialization functions
> and data. To avoid truncating lines, the struct mpc_i2c_match_data has
> been renamed to mpc_i2c_data, which is even the better name.
> 
> Signed-off-by: Wolfgang Grandegger <wg@denx.de>
> ---
>  drivers/i2c/busses/i2c-mpc.c |   99 +++++++++++++++++++----------------------
>  1 files changed, 46 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index f627001..2cb864e 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -66,7 +66,7 @@ struct mpc_i2c_divider {
>  	u16 fdr;	/* including dfsrr */
>  };
>  
> -struct mpc_i2c_match_data {
> +struct mpc_i2c_data {
>  	void (*setclock)(struct device_node *node,
>  			 struct mpc_i2c *i2c,
>  			 u32 clock, u32 prescaler);
> @@ -165,7 +165,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
>  }
>  
>  #ifdef CONFIG_PPC_MPC52xx
> -static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
>  	{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
>  	{28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
>  	{36, 0x26}, {40, 0x27}, {44, 0x04}, {48, 0x28},
> @@ -186,7 +186,8 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
>  	{10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f}
>  };
>  
> -int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
> +static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
> +					  int prescaler)
>  {
>  	const struct mpc_i2c_divider *div = NULL;
>  	unsigned int pvr = mfspr(SPRN_PVR);
> @@ -215,9 +216,9 @@ int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
>  	return div ? (int)div->fdr : -EINVAL;
>  }
>  
> -static void mpc_i2c_setclock_52xx(struct device_node *node,
> -				  struct mpc_i2c *i2c,
> -				  u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> +					    struct mpc_i2c *i2c,
> +					    u32 clock, u32 prescaler)
>  {
>  	int ret, fdr;
>  
> @@ -230,15 +231,15 @@ static void mpc_i2c_setclock_52xx(struct device_node *node,
>  		dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
>  }
>  #else /* !CONFIG_PPC_MPC52xx */
> -static void mpc_i2c_setclock_52xx(struct device_node *node,
> -				  struct mpc_i2c *i2c,
> -				  u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> +					    struct mpc_i2c *i2c,
> +					    u32 clock, u32 prescaler)
>  {
>  }
>  #endif /* CONFIG_PPC_MPC52xx*/
>  
>  #ifdef CONFIG_FSL_SOC
> -static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
>  	{160, 0x0120}, {192, 0x0121}, {224, 0x0122}, {256, 0x0123},
>  	{288, 0x0100}, {320, 0x0101}, {352, 0x0601}, {384, 0x0102},
>  	{416, 0x0602}, {448, 0x0126}, {480, 0x0103}, {512, 0x0127},
> @@ -258,7 +259,7 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
>  	{49152, 0x011e}, {61440, 0x011f}
>  };
>  
> -u32 mpc_i2c_get_sec_cfg_8xxx(void)
> +static u32 __devinit mpc_i2c_get_sec_cfg_8xxx(void)
>  {
>  	struct device_node *node = NULL;
>  	u32 __iomem *reg;
> @@ -287,7 +288,8 @@ u32 mpc_i2c_get_sec_cfg_8xxx(void)
>  	return val;
>  }
>  
> -int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
> +static int __devinit mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
> +					  u32 prescaler)
>  {
>  	const struct mpc_i2c_divider *div = NULL;
>  	u32 divider;
> @@ -320,9 +322,9 @@ int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
>  	return div ? (int)div->fdr : -EINVAL;
>  }
>  
> -static void mpc_i2c_setclock_8xxx(struct device_node *node,
> -				  struct mpc_i2c *i2c,
> -				  u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
> +					    struct mpc_i2c *i2c,
> +					    u32 clock, u32 prescaler)
>  {
>  	int ret, fdr;
>  
> @@ -338,9 +340,9 @@ static void mpc_i2c_setclock_8xxx(struct device_node *node,
>  }
>  
>  #else /* !CONFIG_FSL_SOC */
> -static void mpc_i2c_setclock_8xxx(struct device_node *node,
> -				  struct mpc_i2c *i2c,
> -				  u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
> +					    struct mpc_i2c *i2c,
> +					    u32 clock, u32 prescaler)
>  {
>  }
>  #endif /* CONFIG_FSL_SOC */
> @@ -529,8 +531,8 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
>  			clock = *prop;
>  
>  		if (match->data) {
> -			struct mpc_i2c_match_data *data =
> -				(struct mpc_i2c_match_data *)match->data;
> +			struct mpc_i2c_data *data =
> +				(struct mpc_i2c_data *)match->data;
>  			data->setclock(op->node, i2c, clock, data->prescaler);
>  		} else {
>  			/* Backwards compatibility */
> @@ -582,44 +584,35 @@ static int __devexit fsl_i2c_remove(struct of_device *op)
>  	return 0;
>  };
>  
> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_52xx = {
> +	.setclock = mpc_i2c_setclock_52xx,
> +};
> +
> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_8313 = {
> +	.setclock = mpc_i2c_setclock_8xxx,
> +};
> +
> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_8543 = {
> +	.setclock = mpc_i2c_setclock_8xxx,
> +	.prescaler = 2,
> +};
> +
> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_8544 = {
> +	.setclock = mpc_i2c_setclock_8xxx,
> +	.prescaler = 3,
> +};
> +
>  static const struct of_device_id mpc_i2c_of_match[] = {
> -	{.compatible = "mpc5200-i2c",
> -	 .data = &(struct mpc_i2c_match_data) {
> -			.setclock = mpc_i2c_setclock_52xx,
> -		},
> -	},
> -	{.compatible = "fsl,mpc5200b-i2c",
> -	 .data = &(struct mpc_i2c_match_data) {
> -			.setclock = mpc_i2c_setclock_52xx,
> -		},
> -	},
> -	{.compatible = "fsl,mpc5200-i2c",
> -	 .data = &(struct mpc_i2c_match_data) {
> -			.setclock = mpc_i2c_setclock_52xx,
> -		},
> -	},
> -	{.compatible = "fsl,mpc8313-i2c",
> -	 .data = &(struct mpc_i2c_match_data) {
> -			.setclock = mpc_i2c_setclock_8xxx,
> -		},
> -	},
> -	{.compatible = "fsl,mpc8543-i2c",
> -	 .data = &(struct mpc_i2c_match_data) {
> -			.setclock = mpc_i2c_setclock_8xxx,
> -			.prescaler = 2,
> -		},
> -	},
> -	{.compatible = "fsl,mpc8544-i2c",
> -	 .data = &(struct mpc_i2c_match_data) {
> -			.setclock = mpc_i2c_setclock_8xxx,
> -			.prescaler = 3,
> -		},
> +	{.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
> +	{.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
> +	{.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
> +	{.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
> +	{.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
> +	{.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
>  	/* Backward compatibility */
> -	},
>  	{.compatible = "fsl-i2c", },
>  	{},
>  };

Any particular reason you decided to move this all about?

Are you sure that __devinitdata is the right thing here, I've no idea
if there is currently any hotplug type support for openfirmware or
not.

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Re: [PATCH v2 1/3] i2c-mpc: use __devinit[data] for initialization functions and data

[Wolfgang Grandegger]

Ben Dooks wrote:
> On Mon, Jan 25, 2010 at 09:55:04PM +0100, Wolfgang Grandegger wrote:
>> From: Wolfgang Grandegger <wg@denx.de>
>>
>> "__devinit[data]" has not yet been used for all initialization functions
>> and data. To avoid truncating lines, the struct mpc_i2c_match_data has
>> been renamed to mpc_i2c_data, which is even the better name.
>>
>> Signed-off-by: Wolfgang Grandegger <wg@denx.de>
>> ---
>>  drivers/i2c/busses/i2c-mpc.c |   99 +++++++++++++++++++----------------------
>>  1 files changed, 46 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
>> index f627001..2cb864e 100644
>> --- a/drivers/i2c/busses/i2c-mpc.c
>> +++ b/drivers/i2c/busses/i2c-mpc.c
>> @@ -66,7 +66,7 @@ struct mpc_i2c_divider {
>>  	u16 fdr;	/* including dfsrr */
>>  };
>>  
>> -struct mpc_i2c_match_data {
>> +struct mpc_i2c_data {
>>  	void (*setclock)(struct device_node *node,
>>  			 struct mpc_i2c *i2c,
>>  			 u32 clock, u32 prescaler);
>> @@ -165,7 +165,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
>>  }
>>  
>>  #ifdef CONFIG_PPC_MPC52xx
>> -static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
>> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
>>  	{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
>>  	{28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
>>  	{36, 0x26}, {40, 0x27}, {44, 0x04}, {48, 0x28},
>> @@ -186,7 +186,8 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
>>  	{10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f}
>>  };
>>  
>> -int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
>> +static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
>> +					  int prescaler)
>>  {
>>  	const struct mpc_i2c_divider *div = NULL;
>>  	unsigned int pvr = mfspr(SPRN_PVR);
>> @@ -215,9 +216,9 @@ int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
>>  	return div ? (int)div->fdr : -EINVAL;
>>  }
>>  
>> -static void mpc_i2c_setclock_52xx(struct device_node *node,
>> -				  struct mpc_i2c *i2c,
>> -				  u32 clock, u32 prescaler)
>> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
>> +					    struct mpc_i2c *i2c,
>> +					    u32 clock, u32 prescaler)
>>  {
>>  	int ret, fdr;
>>  
>> @@ -230,15 +231,15 @@ static void mpc_i2c_setclock_52xx(struct device_node *node,
>>  		dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
>>  }
>>  #else /* !CONFIG_PPC_MPC52xx */
>> -static void mpc_i2c_setclock_52xx(struct device_node *node,
>> -				  struct mpc_i2c *i2c,
>> -				  u32 clock, u32 prescaler)
>> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
>> +					    struct mpc_i2c *i2c,
>> +					    u32 clock, u32 prescaler)
>>  {
>>  }
>>  #endif /* CONFIG_PPC_MPC52xx*/
>>  
>>  #ifdef CONFIG_FSL_SOC
>> -static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
>> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
>>  	{160, 0x0120}, {192, 0x0121}, {224, 0x0122}, {256, 0x0123},
>>  	{288, 0x0100}, {320, 0x0101}, {352, 0x0601}, {384, 0x0102},
>>  	{416, 0x0602}, {448, 0x0126}, {480, 0x0103}, {512, 0x0127},
>> @@ -258,7 +259,7 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
>>  	{49152, 0x011e}, {61440, 0x011f}
>>  };
>>  
>> -u32 mpc_i2c_get_sec_cfg_8xxx(void)
>> +static u32 __devinit mpc_i2c_get_sec_cfg_8xxx(void)
>>  {
>>  	struct device_node *node = NULL;
>>  	u32 __iomem *reg;
>> @@ -287,7 +288,8 @@ u32 mpc_i2c_get_sec_cfg_8xxx(void)
>>  	return val;
>>  }
>>  
>> -int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
>> +static int __devinit mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
>> +					  u32 prescaler)
>>  {
>>  	const struct mpc_i2c_divider *div = NULL;
>>  	u32 divider;
>> @@ -320,9 +322,9 @@ int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
>>  	return div ? (int)div->fdr : -EINVAL;
>>  }
>>  
>> -static void mpc_i2c_setclock_8xxx(struct device_node *node,
>> -				  struct mpc_i2c *i2c,
>> -				  u32 clock, u32 prescaler)
>> +static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
>> +					    struct mpc_i2c *i2c,
>> +					    u32 clock, u32 prescaler)
>>  {
>>  	int ret, fdr;
>>  
>> @@ -338,9 +340,9 @@ static void mpc_i2c_setclock_8xxx(struct device_node *node,
>>  }
>>  
>>  #else /* !CONFIG_FSL_SOC */
>> -static void mpc_i2c_setclock_8xxx(struct device_node *node,
>> -				  struct mpc_i2c *i2c,
>> -				  u32 clock, u32 prescaler)
>> +static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
>> +					    struct mpc_i2c *i2c,
>> +					    u32 clock, u32 prescaler)
>>  {
>>  }
>>  #endif /* CONFIG_FSL_SOC */
>> @@ -529,8 +531,8 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
>>  			clock = *prop;
>>  
>>  		if (match->data) {
>> -			struct mpc_i2c_match_data *data =
>> -				(struct mpc_i2c_match_data *)match->data;
>> +			struct mpc_i2c_data *data =
>> +				(struct mpc_i2c_data *)match->data;
>>  			data->setclock(op->node, i2c, clock, data->prescaler);
>>  		} else {
>>  			/* Backwards compatibility */
>> @@ -582,44 +584,35 @@ static int __devexit fsl_i2c_remove(struct of_device *op)
>>  	return 0;
>>  };
>>  
>> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_52xx = {
>> +	.setclock = mpc_i2c_setclock_52xx,
>> +};
>> +
>> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_8313 = {
>> +	.setclock = mpc_i2c_setclock_8xxx,
>> +};
>> +
>> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_8543 = {
>> +	.setclock = mpc_i2c_setclock_8xxx,
>> +	.prescaler = 2,
>> +};
>> +
>> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_8544 = {
>> +	.setclock = mpc_i2c_setclock_8xxx,
>> +	.prescaler = 3,
>> +};
>> +
>>  static const struct of_device_id mpc_i2c_of_match[] = {
>> -	{.compatible = "mpc5200-i2c",
>> -	 .data = &(struct mpc_i2c_match_data) {
>> -			.setclock = mpc_i2c_setclock_52xx,
>> -		},
>> -	},
>> -	{.compatible = "fsl,mpc5200b-i2c",
>> -	 .data = &(struct mpc_i2c_match_data) {
>> -			.setclock = mpc_i2c_setclock_52xx,
>> -		},
>> -	},
>> -	{.compatible = "fsl,mpc5200-i2c",
>> -	 .data = &(struct mpc_i2c_match_data) {
>> -			.setclock = mpc_i2c_setclock_52xx,
>> -		},
>> -	},
>> -	{.compatible = "fsl,mpc8313-i2c",
>> -	 .data = &(struct mpc_i2c_match_data) {
>> -			.setclock = mpc_i2c_setclock_8xxx,
>> -		},
>> -	},
>> -	{.compatible = "fsl,mpc8543-i2c",
>> -	 .data = &(struct mpc_i2c_match_data) {
>> -			.setclock = mpc_i2c_setclock_8xxx,
>> -			.prescaler = 2,
>> -		},
>> -	},
>> -	{.compatible = "fsl,mpc8544-i2c",
>> -	 .data = &(struct mpc_i2c_match_data) {
>> -			.setclock = mpc_i2c_setclock_8xxx,
>> -			.prescaler = 3,
>> -		},
>> +	{.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
>> +	{.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
>> +	{.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
>> +	{.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
>> +	{.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
>> +	{.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
>>  	/* Backward compatibility */
>> -	},
>>  	{.compatible = "fsl-i2c", },
>>  	{},
>>  };
> 
> Any particular reason you decided to move this all about?

This was necessary to allow using __devinit[data] for the clock setup
functions and clock diviver arrays above which results in some notable
saving of memory space if the driver is statically linked into the
kernel. If the data is defined within "mpc_i2c_of_match", section
mismatches are reported.

> Are you sure that __devinitdata is the right thing here, I've no idea
> if there is currently any hotplug type support for openfirmware or
> not.

I agree that __init[data] is more appropriate for this driver even if
many other non-hotplugable drivers use __devinit[data]. I will change that.

Wolfgang.

Re: [PATCH v2 1/3] i2c-mpc: use __devinit[data] for initialization functions and data

[Ben Dooks]

On Tue, Jan 26, 2010 at 07:44:10PM +0100, Wolfgang Grandegger wrote:
> Ben Dooks wrote:
> > On Mon, Jan 25, 2010 at 09:55:04PM +0100, Wolfgang Grandegger wrote:
> >> From: Wolfgang Grandegger <wg@denx.de>
> >>
> >> "__devinit[data]" has not yet been used for all initialization functions
> >> and data. To avoid truncating lines, the struct mpc_i2c_match_data has
> >> been renamed to mpc_i2c_data, which is even the better name.
> >>
> >> Signed-off-by: Wolfgang Grandegger <wg@denx.de>
> >> ---
> >>  drivers/i2c/busses/i2c-mpc.c |   99 +++++++++++++++++++----------------------
> >>  1 files changed, 46 insertions(+), 53 deletions(-)
> >>
> >> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> >> index f627001..2cb864e 100644
> >> --- a/drivers/i2c/busses/i2c-mpc.c
> >> +++ b/drivers/i2c/busses/i2c-mpc.c
> >> @@ -66,7 +66,7 @@ struct mpc_i2c_divider {
> >>  	u16 fdr;	/* including dfsrr */
> >>  };
> >>  
> >> -struct mpc_i2c_match_data {
> >> +struct mpc_i2c_data {
> >>  	void (*setclock)(struct device_node *node,
> >>  			 struct mpc_i2c *i2c,
> >>  			 u32 clock, u32 prescaler);
> >> @@ -165,7 +165,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
> >>  }
> >>  
> >>  #ifdef CONFIG_PPC_MPC52xx
> >> -static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
> >> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
> >>  	{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
> >>  	{28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
> >>  	{36, 0x26}, {40, 0x27}, {44, 0x04}, {48, 0x28},
> >> @@ -186,7 +186,8 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
> >>  	{10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f}
> >>  };
> >>  
> >> -int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
> >> +static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
> >> +					  int prescaler)
> >>  {
> >>  	const struct mpc_i2c_divider *div = NULL;
> >>  	unsigned int pvr = mfspr(SPRN_PVR);
> >> @@ -215,9 +216,9 @@ int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler)
> >>  	return div ? (int)div->fdr : -EINVAL;
> >>  }
> >>  
> >> -static void mpc_i2c_setclock_52xx(struct device_node *node,
> >> -				  struct mpc_i2c *i2c,
> >> -				  u32 clock, u32 prescaler)
> >> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> >> +					    struct mpc_i2c *i2c,
> >> +					    u32 clock, u32 prescaler)
> >>  {
> >>  	int ret, fdr;
> >>  
> >> @@ -230,15 +231,15 @@ static void mpc_i2c_setclock_52xx(struct device_node *node,
> >>  		dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
> >>  }
> >>  #else /* !CONFIG_PPC_MPC52xx */
> >> -static void mpc_i2c_setclock_52xx(struct device_node *node,
> >> -				  struct mpc_i2c *i2c,
> >> -				  u32 clock, u32 prescaler)
> >> +static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> >> +					    struct mpc_i2c *i2c,
> >> +					    u32 clock, u32 prescaler)
> >>  {
> >>  }
> >>  #endif /* CONFIG_PPC_MPC52xx*/
> >>  
> >>  #ifdef CONFIG_FSL_SOC
> >> -static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
> >> +static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
> >>  	{160, 0x0120}, {192, 0x0121}, {224, 0x0122}, {256, 0x0123},
> >>  	{288, 0x0100}, {320, 0x0101}, {352, 0x0601}, {384, 0x0102},
> >>  	{416, 0x0602}, {448, 0x0126}, {480, 0x0103}, {512, 0x0127},
> >> @@ -258,7 +259,7 @@ static const struct mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
> >>  	{49152, 0x011e}, {61440, 0x011f}
> >>  };
> >>  
> >> -u32 mpc_i2c_get_sec_cfg_8xxx(void)
> >> +static u32 __devinit mpc_i2c_get_sec_cfg_8xxx(void)
> >>  {
> >>  	struct device_node *node = NULL;
> >>  	u32 __iomem *reg;
> >> @@ -287,7 +288,8 @@ u32 mpc_i2c_get_sec_cfg_8xxx(void)
> >>  	return val;
> >>  }
> >>  
> >> -int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
> >> +static int __devinit mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
> >> +					  u32 prescaler)
> >>  {
> >>  	const struct mpc_i2c_divider *div = NULL;
> >>  	u32 divider;
> >> @@ -320,9 +322,9 @@ int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler)
> >>  	return div ? (int)div->fdr : -EINVAL;
> >>  }
> >>  
> >> -static void mpc_i2c_setclock_8xxx(struct device_node *node,
> >> -				  struct mpc_i2c *i2c,
> >> -				  u32 clock, u32 prescaler)
> >> +static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
> >> +					    struct mpc_i2c *i2c,
> >> +					    u32 clock, u32 prescaler)
> >>  {
> >>  	int ret, fdr;
> >>  
> >> @@ -338,9 +340,9 @@ static void mpc_i2c_setclock_8xxx(struct device_node *node,
> >>  }
> >>  
> >>  #else /* !CONFIG_FSL_SOC */
> >> -static void mpc_i2c_setclock_8xxx(struct device_node *node,
> >> -				  struct mpc_i2c *i2c,
> >> -				  u32 clock, u32 prescaler)
> >> +static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
> >> +					    struct mpc_i2c *i2c,
> >> +					    u32 clock, u32 prescaler)
> >>  {
> >>  }
> >>  #endif /* CONFIG_FSL_SOC */
> >> @@ -529,8 +531,8 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
> >>  			clock = *prop;
> >>  
> >>  		if (match->data) {
> >> -			struct mpc_i2c_match_data *data =
> >> -				(struct mpc_i2c_match_data *)match->data;
> >> +			struct mpc_i2c_data *data =
> >> +				(struct mpc_i2c_data *)match->data;
> >>  			data->setclock(op->node, i2c, clock, data->prescaler);
> >>  		} else {
> >>  			/* Backwards compatibility */
> >> @@ -582,44 +584,35 @@ static int __devexit fsl_i2c_remove(struct of_device *op)
> >>  	return 0;
> >>  };
> >>  
> >> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_52xx = {
> >> +	.setclock = mpc_i2c_setclock_52xx,
> >> +};
> >> +
> >> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_8313 = {
> >> +	.setclock = mpc_i2c_setclock_8xxx,
> >> +};
> >> +
> >> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_8543 = {
> >> +	.setclock = mpc_i2c_setclock_8xxx,
> >> +	.prescaler = 2,
> >> +};
> >> +
> >> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_8544 = {
> >> +	.setclock = mpc_i2c_setclock_8xxx,
> >> +	.prescaler = 3,
> >> +};
> >> +
> >>  static const struct of_device_id mpc_i2c_of_match[] = {
> >> -	{.compatible = "mpc5200-i2c",
> >> -	 .data = &(struct mpc_i2c_match_data) {
> >> -			.setclock = mpc_i2c_setclock_52xx,
> >> -		},
> >> -	},
> >> -	{.compatible = "fsl,mpc5200b-i2c",
> >> -	 .data = &(struct mpc_i2c_match_data) {
> >> -			.setclock = mpc_i2c_setclock_52xx,
> >> -		},
> >> -	},
> >> -	{.compatible = "fsl,mpc5200-i2c",
> >> -	 .data = &(struct mpc_i2c_match_data) {
> >> -			.setclock = mpc_i2c_setclock_52xx,
> >> -		},
> >> -	},
> >> -	{.compatible = "fsl,mpc8313-i2c",
> >> -	 .data = &(struct mpc_i2c_match_data) {
> >> -			.setclock = mpc_i2c_setclock_8xxx,
> >> -		},
> >> -	},
> >> -	{.compatible = "fsl,mpc8543-i2c",
> >> -	 .data = &(struct mpc_i2c_match_data) {
> >> -			.setclock = mpc_i2c_setclock_8xxx,
> >> -			.prescaler = 2,
> >> -		},
> >> -	},
> >> -	{.compatible = "fsl,mpc8544-i2c",
> >> -	 .data = &(struct mpc_i2c_match_data) {
> >> -			.setclock = mpc_i2c_setclock_8xxx,
> >> -			.prescaler = 3,
> >> -		},
> >> +	{.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
> >> +	{.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
> >> +	{.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
> >> +	{.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
> >> +	{.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
> >> +	{.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
> >>  	/* Backward compatibility */
> >> -	},
> >>  	{.compatible = "fsl-i2c", },
> >>  	{},
> >>  };
> > 
> > Any particular reason you decided to move this all about?
> 
> This was necessary to allow using __devinit[data] for the clock setup
> functions and clock diviver arrays above which results in some notable
> saving of memory space if the driver is statically linked into the
> kernel. If the data is defined within "mpc_i2c_of_match", section
> mismatches are reported.
> 
> > Are you sure that __devinitdata is the right thing here, I've no idea
> > if there is currently any hotplug type support for openfirmware or
> > not.
> 
> I agree that __init[data] is more appropriate for this driver even if
> many other non-hotplugable drivers use __devinit[data]. I will change that.

sorry, may have gotten confused by which type of __init is which, I
think __devinit is the correct one here.

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Re: [PATCH v2 1/3] i2c-mpc: use __devinit[data] for initialization functions and data

[Wolfgang Grandegger]

Ben Dooks wrote:
> On Tue, Jan 26, 2010 at 07:44:10PM +0100, Wolfgang Grandegger wrote:
>> Ben Dooks wrote:
>>> On Mon, Jan 25, 2010 at 09:55:04PM +0100, Wolfgang Grandegger wrote:
[snip]
>>> Any particular reason you decided to move this all about?
>> This was necessary to allow using __devinit[data] for the clock setup
>> functions and clock diviver arrays above which results in some notable
>> saving of memory space if the driver is statically linked into the
>> kernel. If the data is defined within "mpc_i2c_of_match", section
>> mismatches are reported.
>>
>>> Are you sure that __devinitdata is the right thing here, I've no idea
>>> if there is currently any hotplug type support for openfirmware or
>>> not.
>> I agree that __init[data] is more appropriate for this driver even if
>> many other non-hotplugable drivers use __devinit[data]. I will change that.
> 
> sorry, may have gotten confused by which type of __init is which, I
> think __devinit is the correct one here.

Why? It's not obvious to me how a i2c device might be hotpugged? Will
require v4 to fix it.

Thanks,

Wolfgang.