<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=us-ascii" http-equiv="Content-Type">
<title></title>
</head>
<body bgcolor="#ffffff" text="#000000">
Olof Johansson wrote:
<blockquote cite="mid20070423025709.GA28805@lixom.net" type="cite">
<pre wrap="">On Fri, Apr 20, 2007 at 08:27:14AM +0400, Vitaly Bordug wrote:
</pre>
<blockquote type="cite">
<pre wrap="">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
</pre>
</blockquote>
<pre wrap=""><!---->
Does it hurt to always do it, even when the driver is not enabled? THat'd
do away with an ifdef.
</pre>
</blockquote>
Well it will hurt - 8xx has conflicting io pin configurations, and
nothing should be set up "just in case".<br>
<br>
<blockquote cite="mid20070423025709.GA28805@lixom.net" type="cite">
<pre wrap="">Also, if you move the static function up, you don't need a prototype. That
goes for other stuff in this file too.
</pre>
<blockquote type="cite">
<pre wrap=""> }
@@ -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);
+}
</pre>
</blockquote>
<pre wrap=""><!---->
Looks like you moved this out of the driver and into the platform
code. What happens to other platforms where it's used?
</pre>
</blockquote>
i2c && 8xx combo never work with 2.6 at least in mainstream.
That's why related stuff were scheduled to removal by Jean even,<br>
before I came up with this stuff.<br>
<blockquote cite="mid20070423025709.GA28805@lixom.net" type="cite">
<pre wrap=""></pre>
<blockquote type="cite">
<pre wrap="">+
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];
</pre>
</blockquote>
<pre wrap=""><!---->
Why? No code that uses it has been changed. Is it a bugfix?
</pre>
</blockquote>
maybe something stray...<br>
<blockquote cite="mid20070423025709.GA28805@lixom.net" type="cite">
<pre wrap=""></pre>
<blockquote type="cite">
<pre wrap=""> 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);
</pre>
</blockquote>
<pre wrap=""><!---->
This could all be done with an of_platform driver instead, and avoid the above.
(Someone else already suggested that I believe).
</pre>
</blockquote>
I know i know. But it was decided, while both ppc/ and powerpc/ wander
around, platform devices way is preferrable.<br>
It is apparent why - so far only mpc885 is alive in arch/powerpc, and
it is not going to change soon for 8xx. OTOH,<br>
some stuff from arch/ppc might use it/add BSP configuration etc.<br>
<br>
Having some devices on of_device and some on pdev look kinda messy.<br>
<blockquote cite="mid20070423025709.GA28805@lixom.net" type="cite">
<pre wrap=""></pre>
</blockquote>
Thanks,<br>
Vitaly<br>
<br>
</body>
</html>