* [PATCH] Convert i2c-mpc from a platform driver into a of_platform driver, V2
From: Jon Smirl @ 2008-06-29 16:44 UTC (permalink / raw)
To: i2c, Linuxppc-dev
This version adds of_find_i2c_device_by_node() which is needed by ASOC drivers. To make it work I had to make the i2c bus struct visible. It also picks up a couple of other minor fixes that have been posted to the lists.
Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
arch/powerpc/sysdev/fsl_soc.c | 122 -----------------------------------------
drivers/i2c/busses/i2c-mpc.c | 104 ++++++++++++++++++++---------------
drivers/i2c/i2c-core.c | 2 -
drivers/of/of_i2c.c | 37 +++++++++---
include/linux/i2c.h | 3 +
include/linux/of_i2c.h | 2 +
6 files changed, 92 insertions(+), 178 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 019657c..ebcec73 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -414,128 +414,6 @@ err:
arch_initcall(gfar_of_init);
-#ifdef CONFIG_I2C_BOARDINFO
-#include <linux/i2c.h>
-struct i2c_driver_device {
- char *of_device;
- char *i2c_type;
-};
-
-static struct i2c_driver_device i2c_devices[] __initdata = {
- {"ricoh,rs5c372a", "rs5c372a"},
- {"ricoh,rs5c372b", "rs5c372b"},
- {"ricoh,rv5c386", "rv5c386"},
- {"ricoh,rv5c387a", "rv5c387a"},
- {"dallas,ds1307", "ds1307"},
- {"dallas,ds1337", "ds1337"},
- {"dallas,ds1338", "ds1338"},
- {"dallas,ds1339", "ds1339"},
- {"dallas,ds1340", "ds1340"},
- {"stm,m41t00", "m41t00"},
- {"dallas,ds1374", "ds1374"},
-};
-
-static int __init of_find_i2c_driver(struct device_node *node,
- struct i2c_board_info *info)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(i2c_devices); i++) {
- if (!of_device_is_compatible(node, i2c_devices[i].of_device))
- continue;
- if (strlcpy(info->type, i2c_devices[i].i2c_type,
- I2C_NAME_SIZE) >= I2C_NAME_SIZE)
- return -ENOMEM;
- return 0;
- }
- return -ENODEV;
-}
-
-static void __init of_register_i2c_devices(struct device_node *adap_node,
- int bus_num)
-{
- struct device_node *node = NULL;
-
- while ((node = of_get_next_child(adap_node, node))) {
- struct i2c_board_info info = {};
- const u32 *addr;
- int len;
-
- addr = of_get_property(node, "reg", &len);
- if (!addr || len < sizeof(int) || *addr > (1 << 10) - 1) {
- printk(KERN_WARNING "fsl_soc.c: invalid i2c device entry\n");
- continue;
- }
-
- info.irq = irq_of_parse_and_map(node, 0);
- if (info.irq == NO_IRQ)
- info.irq = -1;
-
- if (of_find_i2c_driver(node, &info) < 0)
- continue;
-
- info.addr = *addr;
-
- i2c_register_board_info(bus_num, &info, 1);
- }
-}
-
-static int __init fsl_i2c_of_init(void)
-{
- struct device_node *np;
- unsigned int i = 0;
- struct platform_device *i2c_dev;
- int ret;
-
- for_each_compatible_node(np, NULL, "fsl-i2c") {
- struct resource r[2];
- struct fsl_i2c_platform_data i2c_data;
- const unsigned char *flags = NULL;
-
- 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;
-
- of_irq_to_resource(np, 0, &r[1]);
-
- i2c_dev = platform_device_register_simple("fsl-i2c", i, r, 2);
- if (IS_ERR(i2c_dev)) {
- ret = PTR_ERR(i2c_dev);
- goto err;
- }
-
- i2c_data.device_flags = 0;
- flags = of_get_property(np, "dfsrr", NULL);
- if (flags)
- i2c_data.device_flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
-
- flags = of_get_property(np, "fsl5200-clocking", NULL);
- if (flags)
- i2c_data.device_flags |= FSL_I2C_DEV_CLOCK_5200;
-
- ret =
- platform_device_add_data(i2c_dev, &i2c_data,
- sizeof(struct
- fsl_i2c_platform_data));
- if (ret)
- goto unreg;
-
- of_register_i2c_devices(np, i++);
- }
-
- return 0;
-
-unreg:
- platform_device_unregister(i2c_dev);
-err:
- return ret;
-}
-
-arch_initcall(fsl_i2c_of_init);
-#endif
#ifdef CONFIG_PPC_83xx
static int __init mpc83xx_wdt_init(void)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index a076129..36bea49 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -17,7 +17,8 @@
#include <linux/module.h>
#include <linux/sched.h>
#include <linux/init.h>
-#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_i2c.h>
#include <asm/io.h>
#include <linux/fsl_devices.h>
@@ -25,13 +26,13 @@
#include <linux/interrupt.h>
#include <linux/delay.h>
-#define MPC_I2C_ADDR 0x00
+#define DRV_NAME "mpc-i2c"
+
#define MPC_I2C_FDR 0x04
#define MPC_I2C_CR 0x08
#define MPC_I2C_SR 0x0c
#define MPC_I2C_DR 0x10
#define MPC_I2C_DFSRR 0x14
-#define MPC_I2C_REGION 0x20
#define CCR_MEN 0x80
#define CCR_MIEN 0x40
@@ -315,102 +316,117 @@ static struct i2c_adapter mpc_ops = {
.timeout = 1,
};
-static int fsl_i2c_probe(struct platform_device *pdev)
+static int __devinit fsl_i2c_probe(struct of_device *op, const struct of_device_id *match)
{
int result = 0;
struct mpc_i2c *i2c;
- struct fsl_i2c_platform_data *pdata;
- struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
- pdata = (struct fsl_i2c_platform_data *) pdev->dev.platform_data;
i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
if (!i2c)
return -ENOMEM;
- i2c->irq = platform_get_irq(pdev, 0);
- if (i2c->irq < 0)
- i2c->irq = NO_IRQ; /* Use polling */
+ if (of_get_property(op->node, "dfsrr", NULL))
+ i2c->flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
- i2c->flags = pdata->device_flags;
- init_waitqueue_head(&i2c->queue);
+ if (of_device_is_compatible(op->node, "fsl,mpc5200-i2c") ||
+ of_device_is_compatible(op->node, "mpc5200-i2c"))
+ i2c->flags |= FSL_I2C_DEV_CLOCK_5200;
- i2c->base = ioremap((phys_addr_t)r->start, MPC_I2C_REGION);
+ init_waitqueue_head(&i2c->queue);
+ i2c->base = of_iomap(op->node, 0);
if (!i2c->base) {
printk(KERN_ERR "i2c-mpc - failed to map controller\n");
result = -ENOMEM;
goto fail_map;
}
- if (i2c->irq != NO_IRQ)
- if ((result = request_irq(i2c->irq, mpc_i2c_isr,
- IRQF_SHARED, "i2c-mpc", i2c)) < 0) {
- printk(KERN_ERR
- "i2c-mpc - failed to attach interrupt\n");
- goto fail_irq;
+ i2c->irq = irq_of_parse_and_map(op->node, 0);
+ if (i2c->irq != NO_IRQ) { /* i2c->irq = NO_IRQ implies polling */
+ result = request_irq(i2c->irq, mpc_i2c_isr,
+ IRQF_SHARED, "i2c-mpc", i2c);
+ if (result < 0) {
+ printk(KERN_ERR "i2c-mpc - failed to attach interrupt\n");
+ goto fail_request;
}
-
+ }
+
mpc_i2c_setclock(i2c);
- platform_set_drvdata(pdev, i2c);
+
+ dev_set_drvdata(&op->dev, i2c);
i2c->adap = mpc_ops;
- i2c->adap.nr = pdev->id;
i2c_set_adapdata(&i2c->adap, i2c);
- i2c->adap.dev.parent = &pdev->dev;
- if ((result = i2c_add_numbered_adapter(&i2c->adap)) < 0) {
+ i2c->adap.dev.parent = &op->dev;
+
+ result = i2c_add_adapter(&i2c->adap);
+ if (result < 0) {
printk(KERN_ERR "i2c-mpc - failed to add adapter\n");
goto fail_add;
}
+ of_register_i2c_devices(&i2c->adap, op->node);
return result;
- fail_add:
- if (i2c->irq != NO_IRQ)
- free_irq(i2c->irq, i2c);
- fail_irq:
- iounmap(i2c->base);
- fail_map:
+ fail_add:
+ dev_set_drvdata(&op->dev, NULL);
+ free_irq(i2c->irq, i2c);
+ fail_request:
+ irq_dispose_mapping(i2c->irq);
+ iounmap(i2c->base);
+ fail_map:
kfree(i2c);
return result;
};
-static int fsl_i2c_remove(struct platform_device *pdev)
+static int __devexit fsl_i2c_remove(struct of_device *op)
{
- struct mpc_i2c *i2c = platform_get_drvdata(pdev);
+ struct mpc_i2c *i2c = dev_get_drvdata(&op->dev);
i2c_del_adapter(&i2c->adap);
- platform_set_drvdata(pdev, NULL);
+ dev_set_drvdata(&op->dev, NULL);
if (i2c->irq != NO_IRQ)
free_irq(i2c->irq, i2c);
+ irq_dispose_mapping(i2c->irq);
iounmap(i2c->base);
kfree(i2c);
return 0;
};
-/* work with hotplug and coldplug */
-MODULE_ALIAS("platform:fsl-i2c");
+static const struct of_device_id mpc_i2c_of_match[] = {
+ {.compatible = "fsl-i2c",},
+ {},
+};
+MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
+
/* Structure for a device driver */
-static struct platform_driver fsl_i2c_driver = {
- .probe = fsl_i2c_probe,
- .remove = fsl_i2c_remove,
- .driver = {
- .owner = THIS_MODULE,
- .name = "fsl-i2c",
+static struct of_platform_driver mpc_i2c_driver = {
+ .match_table = mpc_i2c_of_match,
+ .probe = fsl_i2c_probe,
+ .remove = __devexit_p(fsl_i2c_remove),
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = DRV_NAME,
},
};
static int __init fsl_i2c_init(void)
{
- return platform_driver_register(&fsl_i2c_driver);
+ int rv;
+
+ rv = of_register_platform_driver(&mpc_i2c_driver);
+ if (rv)
+ printk(KERN_ERR DRV_NAME
+ " of_register_platform_driver failed (%i)\n", rv);
+ return rv;
}
static void __exit fsl_i2c_exit(void)
{
- platform_driver_unregister(&fsl_i2c_driver);
+ of_unregister_platform_driver(&mpc_i2c_driver);
}
module_init(fsl_i2c_init);
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d0175f4..e3abe1b 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -208,7 +208,7 @@ static struct device_attribute i2c_dev_attrs[] = {
{ },
};
-static struct bus_type i2c_bus_type = {
+struct bus_type i2c_bus_type = {
.name = "i2c",
.dev_attrs = i2c_dev_attrs,
.match = i2c_device_match,
diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
index b2ccdcb..67cbdc7 100644
--- a/drivers/of/of_i2c.c
+++ b/drivers/of/of_i2c.c
@@ -74,7 +74,7 @@ static int of_find_i2c_driver(struct device_node *node,
void of_register_i2c_devices(struct i2c_adapter *adap,
struct device_node *adap_node)
{
- void *result;
+ struct i2c_client *i2c_dev;
struct device_node *node;
for_each_child_of_node(adap_node, node) {
@@ -89,29 +89,44 @@ void of_register_i2c_devices(struct i2c_adapter *adap,
continue;
}
- info.irq = irq_of_parse_and_map(node, 0);
- if (info.irq == NO_IRQ)
- info.irq = -1;
-
- if (of_find_i2c_driver(node, &info) < 0) {
- irq_dispose_mapping(info.irq);
+ if (of_find_i2c_driver(node, &info) < 0)
continue;
- }
+ info.irq = irq_of_parse_and_map(node, 0);
info.addr = *addr;
- request_module(info.type);
+ request_module("%s", info.type);
- result = i2c_new_device(adap, &info);
- if (result == NULL) {
+ i2c_dev = i2c_new_device(adap, &info);
+ if (i2c_dev == NULL) {
printk(KERN_ERR
"of-i2c: Failed to load driver for %s\n",
info.type);
irq_dispose_mapping(info.irq);
continue;
}
+
+ i2c_dev->dev.archdata.of_node = of_node_get(node);
}
}
EXPORT_SYMBOL(of_register_i2c_devices);
+static int of_dev_node_match(struct device *dev, void *data)
+{
+ return dev->archdata.of_node == data;
+}
+
+struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
+{
+ struct device *dev;
+
+ dev = bus_find_device(&i2c_bus_type, NULL, node,
+ of_dev_node_match);
+ if (!dev)
+ return NULL;
+
+ return to_i2c_client(dev);
+}
+EXPORT_SYMBOL(of_find_i2c_device_by_node);
+
MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index fb9af6a..186b22d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -92,6 +92,9 @@ extern s32 i2c_smbus_read_i2c_block_data(struct i2c_client * client,
extern s32 i2c_smbus_write_i2c_block_data(struct i2c_client * client,
u8 command, u8 length,
const u8 *values);
+
+/* Base of the i2c bus */
+extern struct bus_type i2c_bus_type;
/*
* A driver is capable of handling one or more physical devices present on
diff --git a/include/linux/of_i2c.h b/include/linux/of_i2c.h
index bd2a870..17d5897 100644
--- a/include/linux/of_i2c.h
+++ b/include/linux/of_i2c.h
@@ -16,5 +16,7 @@
void of_register_i2c_devices(struct i2c_adapter *adap,
struct device_node *adap_node);
+struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
+
#endif /* __LINUX_OF_I2C_H */
^ permalink raw reply related
* Re: [i2c] [PATCH] Convert i2c-mpc from a platform driver to an of_platform one
From: Jean Delvare @ 2008-06-29 16:35 UTC (permalink / raw)
To: Sean MacLennan
Cc: David Brownell, Wolfram Sang, linuxppc-dev list, Linux,
Timur Tabi, I2C
In-Reply-To: <20080629122439.6295d63c@lappy.seanm.ca>
On Sun, 29 Jun 2008 12:24:39 -0400, Sean MacLennan wrote:
> On Sun, 29 Jun 2008 09:17:25 +0200
> "Jean Delvare" <khali@linux-fr.org> wrote:
>
> > Ah, OK. If you use i2c_new_device() then it's alright.
>
> Correct.
>
> I have done the same thing for the i2c-ibm_iic.c driver. Jean, I think
> you will like this. It gets rid of the index and the numbered drivers.
> And the walking of the device tree is very clean because the dts knows
> all the devices.
>
> For example here is the relevant portion of the dts for the Warp:
>
> IIC0: i2c@ef600700 {
> compatible = "ibm,iic-440ep", "ibm,iic-440gp", "ibm,iic";
> reg = <ef600700 14>;
> interrupt-parent = <&UIC0>;
> interrupts = <2 4>;
> #address-cells = <1>;
> #size-cells = <0>;
>
> ad7414@4a {
> compatible = "adi,ad7414";
> reg = <4a>;
> interrupts = <19 8>;
> interrupt-parent = <&UIC0>;
> };
> };
>
> It clearly shows that first i2c controller (IIC0) contains one ad7414 device at address 4A.
That's fine with me. I expected the dts to be converted to platform
initialization data (i2c_board_info structures) being registered with
i2c_register_board_info() and numbered adapters. But if you prefer
unnumbered adapters and the platform code or the bus driver itself
calls i2c_new_device() based on the dts, that should work too.
--
Jean Delvare
^ permalink raw reply
* Re: [i2c] [PATCH] Convert i2c-mpc from a platform driver to an of_platform one
From: Sean MacLennan @ 2008-06-29 16:24 UTC (permalink / raw)
To: Jean Delvare
Cc: David Brownell, Wolfram Sang, linuxppc-dev list, Linux,
Timur Tabi, I2C
In-Reply-To: <20080629091725.291974e9@hyperion.delvare>
On Sun, 29 Jun 2008 09:17:25 +0200
"Jean Delvare" <khali@linux-fr.org> wrote:
> Ah, OK. If you use i2c_new_device() then it's alright.
Correct.
I have done the same thing for the i2c-ibm_iic.c driver. Jean, I think
you will like this. It gets rid of the index and the numbered drivers.
And the walking of the device tree is very clean because the dts knows
all the devices.
For example here is the relevant portion of the dts for the Warp:
IIC0: i2c@ef600700 {
compatible = "ibm,iic-440ep", "ibm,iic-440gp", "ibm,iic";
reg = <ef600700 14>;
interrupt-parent = <&UIC0>;
interrupts = <2 4>;
#address-cells = <1>;
#size-cells = <0>;
ad7414@4a {
compatible = "adi,ad7414";
reg = <4a>;
interrupts = <19 8>;
interrupt-parent = <&UIC0>;
};
};
It clearly shows that first i2c controller (IIC0) contains one ad7414 device at address 4A.
Cheers,
Sean
^ permalink raw reply
* Re: [PATCH 3/8][Version 2] MPC5121 Add generic board support
From: Grant Likely @ 2008-06-29 8:01 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <1214342672-23536-4-git-send-email-jrigby@freescale.com>
On Tue, Jun 24, 2008 at 3:24 PM, John Rigby <jrigby@freescale.com> wrote:
> Move shared code from mpc5121_ads.c to mpc512x_shared.c.
> Add new generic board setup mpc5121_generic.c
>
> Signed-off-by: John Rigby <jrigby@freescale.com>
> ---
> +void __init mpc512x_init_IRQ(void)
> +{
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "fsl,ipic");
Along with all my other device-tree conventions comments; you should
really be testing for fsl,mpc5121-ipic here. fsl,ipic is not a good
value for compatible.
g.
^ permalink raw reply
* Re: [i2c] [PATCH] Convert i2c-mpc from a platform driver to an of_platform one
From: Jean Delvare @ 2008-06-29 7:17 UTC (permalink / raw)
To: Grant Likely
Cc: Wolfram Sang, David Brownell, linuxppc-dev list, Linux,
Timur Tabi, I2C
In-Reply-To: <20080629065812.GO13876@secretlab.ca>
On Sun, 29 Jun 2008 00:58:12 -0600, Grant Likely wrote:
> On Sun, Jun 29, 2008 at 08:31:43AM +0200, Jean Delvare wrote:
> > Hi Jon, Grant,
> >
> > On Sat, 28 Jun 2008 22:49:40 -0600, Grant Likely wrote:
> > > On Sat, Jun 28, 2008 at 10:05:28PM -0400, Jon Smirl wrote:
> > > > >
> > > > > The driver was previously using i2c_add_numbered_adapter(), giving MPC
> > > > > platform the possibility to use new-style i2c drivers:
> > > > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=1469fa263870acd890a4b9f6ef557acc5d673b44
> > > > > You're breaking this, I doubt it's on purpose?
> > > >
> > > > Grant, what do you want here? You're the one who converted it to
> > > > i2c_add_numbered_adapter. But in other posts you've said that the
> > > > device tree should have nothing to do with bus numbering.
> > >
> > > Yes, I did make that change, but that was when it was a platform bus
> > > driver. Converting it to an of_platform bus driver entirely changes the
> > > situation and it should go back to using i2c_add_adapter() with a parse
> > > of the device tree for child nodes.
> >
> > I am surprised and disappointed, as this sounds like a regression.
> > Registering the i2c buses with random numbers and parsing the device
> > tree later to figure out which devices are where, is what everybody was
> > doing before the new i2c device/driver matching model was implemented,
> > because there was no other way. I'm curious why you are going back to
> > this approach when i2c-core now offers something way cleaner and more
> > efficient.
>
> Ah, but the whole random number parsing thing is no longer necessary
> because we ensure that registration of i2c devices always occurs
> after the i2c adapter is created (for device tree aware i2c adapter
> drivers. adapters that aren't device tree aware still need to assign a
> number).
>
> After the i2c adapter registers itself, of_register_i2c_devices() is called
> which walks through the child nodes of the i2c adapter node in the device
> tree. Each child node is an i2c device, and it immediately get
> registered with the adapter. Because this ensures that i2c device
> registration always happens after adapter registration, and since the
> pointer to the i2c_adapter is known, then i2c_new_device() can be used
> directly without ever needing to know the bus number.
Ah, OK. If you use i2c_new_device() then it's alright.
--
Jean Delvare
^ permalink raw reply
* Re: [Resend][PATCH 1/8][Version 2] MPC5121 Update MPC5121ADS device tree
From: Grant Likely @ 2008-06-29 7:09 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev, John Rigby
In-Reply-To: <20080629061533.GL13876@secretlab.ca>
On Sun, Jun 29, 2008 at 12:15 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Thu, Jun 26, 2008 at 09:40:57PM -0600, John Rigby wrote:
>> Because get_immrbase in fsl_soc.c does not work without it.
>
> I think I'd rather see get_immrbase() instead then.
er, I think I'd rather see get_immrbase() *fixed* instead then.
g.
^ permalink raw reply
* Re: [i2c] [PATCH] Convert i2c-mpc from a platform driver to an of_platform one
From: Grant Likely @ 2008-06-29 6:58 UTC (permalink / raw)
To: Jean Delvare
Cc: Wolfram Sang, David Brownell, linuxppc-dev list, Timur Tabi,
Linux I2C
In-Reply-To: <20080629083143.7f39b3c1@hyperion.delvare>
On Sun, Jun 29, 2008 at 08:31:43AM +0200, Jean Delvare wrote:
> Hi Jon, Grant,
>
> On Sat, 28 Jun 2008 22:49:40 -0600, Grant Likely wrote:
> > On Sat, Jun 28, 2008 at 10:05:28PM -0400, Jon Smirl wrote:
> > > >
> > > > The driver was previously using i2c_add_numbered_adapter(), giving MPC
> > > > platform the possibility to use new-style i2c drivers:
> > > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=1469fa263870acd890a4b9f6ef557acc5d673b44
> > > > You're breaking this, I doubt it's on purpose?
> > >
> > > Grant, what do you want here? You're the one who converted it to
> > > i2c_add_numbered_adapter. But in other posts you've said that the
> > > device tree should have nothing to do with bus numbering.
> >
> > Yes, I did make that change, but that was when it was a platform bus
> > driver. Converting it to an of_platform bus driver entirely changes the
> > situation and it should go back to using i2c_add_adapter() with a parse
> > of the device tree for child nodes.
>
> I am surprised and disappointed, as this sounds like a regression.
> Registering the i2c buses with random numbers and parsing the device
> tree later to figure out which devices are where, is what everybody was
> doing before the new i2c device/driver matching model was implemented,
> because there was no other way. I'm curious why you are going back to
> this approach when i2c-core now offers something way cleaner and more
> efficient.
Ah, but the whole random number parsing thing is no longer necessary
because we ensure that registration of i2c devices always occurs
after the i2c adapter is created (for device tree aware i2c adapter
drivers. adapters that aren't device tree aware still need to assign a
number).
After the i2c adapter registers itself, of_register_i2c_devices() is called
which walks through the child nodes of the i2c adapter node in the device
tree. Each child node is an i2c device, and it immediately get
registered with the adapter. Because this ensures that i2c device
registration always happens after adapter registration, and since the
pointer to the i2c_adapter is known, then i2c_new_device() can be used
directly without ever needing to know the bus number.
g.
^ permalink raw reply
* Re: [i2c] [PATCH] Convert i2c-mpc from a platform driver to an of_platform one
From: Jean Delvare @ 2008-06-29 6:31 UTC (permalink / raw)
To: Grant Likely
Cc: Wolfram Sang, David Brownell, linuxppc-dev list, Linux,
Timur Tabi, I2C
In-Reply-To: <20080629044940.GC13658@secretlab.ca>
Hi Jon, Grant,
On Sat, 28 Jun 2008 22:49:40 -0600, Grant Likely wrote:
> On Sat, Jun 28, 2008 at 10:05:28PM -0400, Jon Smirl wrote:
> > On 6/25/08, Jean Delvare <khali@linux-fr.org> wrote:
> > > >
> > > > i2c->adap = mpc_ops;
> > > > - i2c->adap.nr = pdev->id;
> > > > i2c_set_adapdata(&i2c->adap, i2c);
> > > > - i2c->adap.dev.parent = &pdev->dev;
> > > > - if ((result = i2c_add_numbered_adapter(&i2c->adap)) < 0) {
> > > > + i2c->adap.dev.parent = &op->dev;
> > > > +
> > > > + result = i2c_add_adapter(&i2c->adap);
> > >
> > >
> > > The driver was previously using i2c_add_numbered_adapter(), giving MPC
> > > platform the possibility to use new-style i2c drivers:
> > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=1469fa263870acd890a4b9f6ef557acc5d673b44
> > > You're breaking this, I doubt it's on purpose?
> >
> > Grant, what do you want here? You're the one who converted it to
> > i2c_add_numbered_adapter. But in other posts you've said that the
> > device tree should have nothing to do with bus numbering.
>
> Yes, I did make that change, but that was when it was a platform bus
> driver. Converting it to an of_platform bus driver entirely changes the
> situation and it should go back to using i2c_add_adapter() with a parse
> of the device tree for child nodes.
I am surprised and disappointed, as this sounds like a regression.
Registering the i2c buses with random numbers and parsing the device
tree later to figure out which devices are where, is what everybody was
doing before the new i2c device/driver matching model was implemented,
because there was no other way. I'm curious why you are going back to
this approach when i2c-core now offers something way cleaner and more
efficient.
But well, you'll know better. I'm not familiar with the platform, and
don't have the time to look into it anyway.
> > Once this driver is converted to an OF one it shouldn't need bus ids
> > since all of the i2c devices will be children of the bus node. We can
> > just let the i2c subsystem assign a bus number.
>
> Exactly.
>
> > Timur has some issues with the i2c bus number in his ALSA driver. The
> > problem is locating the i2c device when the i2s driver loads. Parsing
> > the device tree to extract an i2c bus and device number is not a good
> > solution.
>
> The trick here is to store the pointer to the device node inside the
> i2c device. I do this with SPI devices like this:
>
> /* Store a pointer to the node in the device structure */
> of_node_get(nc);
> spi->dev.archdata.of_node = nc;
>
> Then, when you've got a device_node pointer, you can parse through the
> set of registered SPI devices and match against the one that has the
> same device node pointer.
>
> > codec-handle should give you the i2c device node. But then we can't
> > use of_find_device_by_node because the i2c device is not an of_device,
> > it's a cross platform i2c_device. Should of_find_device_by_node()
> > return a 'struct device' instead of a 'struct of_device' and leave it
> > as a user exercise to cast up? It is used nine times in the kernel,
> > mostly sparc.
>
> No, this doesn't work because I2C and SPI devices are not of_platform
> devices. They aren't even platform devices. of_find_device_by_node()
> only works for the of_platform bus.
>
> Using archdata.of_node is part of the device structure and works for
> every bus type (platform, of_platform, SPI, I2C, PCI, etc.)
--
Jean Delvare
^ permalink raw reply
* Re: [Resend][PATCH 1/8][Version 2] MPC5121 Update MPC5121ADS device tree
From: Grant Likely @ 2008-06-29 6:15 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev, John Rigby
In-Reply-To: <4b73d43f0806262040s7993dcb7vd8a3d05793ea8a7@mail.gmail.com>
On Thu, Jun 26, 2008 at 09:40:57PM -0600, John Rigby wrote:
> On Thu, Jun 26, 2008 at 7:42 PM, David Gibson <david@gibson.dropbear.id.au> wrote:
> > On Wed, Jun 18, 2008 at 02:24:46PM -0600, John Rigby wrote:
> >> soc@80000000 {
> >> compatible = "fsl,mpc5121-immr";
> >> + device_type = "soc";
> >
> > I realise we still need the unwanted device_type value on the soc in
> > some cases for backwards compatibility, but why are you adding it
> > where it wasn't before..?
> >
> Because get_immrbase in fsl_soc.c does not work without it.
I think I'd rather see get_immrbase() instead then.
g.
^ permalink raw reply
* Re: [Resend][PATCH 1/8][Version 2] MPC5121 Update MPC5121ADS device tree
From: Grant Likely @ 2008-06-29 6:12 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <20080625203051.4FE882F123B@rumpole.eng.lineo.com>
On Wed, Jun 18, 2008 at 02:24:46PM -0600, John Rigby wrote:
> Updated device tree for MPC5121ADS
Really should be more detailed in the commit message.
>
> Signed-off-by: John Rigby <jrigby@freescale.com>
> ---
> arch/powerpc/boot/dts/mpc5121ads.dts | 309 ++++++++++++++++++++++++++++++++-
> 1 files changed, 299 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts b/arch/powerpc/boot/dts/mpc5121ads.dts
> index 94ad7b2..67dc920 100644
> --- a/arch/powerpc/boot/dts/mpc5121ads.dts
> +++ b/arch/powerpc/boot/dts/mpc5121ads.dts
> @@ -1,7 +1,7 @@
> /*
> - * MPC5121E MDS Device Tree Source
> + * MPC5121E ADS Device Tree Source
> *
> - * Copyright 2007 Freescale Semiconductor Inc.
> + * Copyright 2007,2008 Freescale Semiconductor Inc.
> *
> * 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
> @@ -17,6 +17,10 @@
> #address-cells = <1>;
> #size-cells = <1>;
>
> + aliases {
> + pci = &pci;
> + };
> +
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -39,6 +43,39 @@
> reg = <0x00000000 0x10000000>; // 256MB at 0
> };
>
> + mbx@20000000 {
> + compatible = "fsl,mpc5121-mbx";
> + reg = <0x20000000 0x4000>;
> + interrupts = <66 0x8>;
> + interrupt-parent = < &ipic >;
> + };
> +
> + sram@30000000 {
> + compatible = "fsl,mpc5121-sram";
> + reg = <0x30000000 0x20000>; // 128K at 0x30000000
> + };
> +
> + nfc@40000000 {
> + compatible = "fsl,mpc5121-nfc";
> + reg = <0x40000000 0x100000>; // 1M at 0x40000000
> + interrupts = <6 8>;
> + interrupt-parent = < &ipic >;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + bank-width = <1>;
> + // ADS has two Hynix 512MB Nand flash chips in a single
> + // stacked package .
> + chips = <2>;
> + nand0@0 {
> + label = "nand0";
> + reg = <0x00000000 0x02000000>; // first 32 MB of chip 0
> + };
> + nand1@20000000 {
> + label = "nand1";
> + reg = <0x20000000 0x02000000>; // first 32 MB of chip 1
> + };
> + };
> +
> localbus@80000020 {
> compatible = "fsl,mpc5121ads-localbus";
> #address-cells = <2>;
> @@ -51,18 +88,56 @@
> flash@0,0 {
> compatible = "cfi-flash";
> reg = <0 0x0 0x4000000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> bank-width = <4>;
> - device-width = <1>;
> + device-width = <2>;
> + protected@0 {
> + label = "protected";
> + reg = <0x00000000 0x00040000>; // first sector is protected
> + read-only;
> + };
> + filesystem@40000 {
> + label = "filesystem";
> + reg = <0x00040000 0x03c00000>; // 60M for filesystem
> + };
> + kernel@3c40000 {
> + label = "kernel";
> + reg = <0x03c40000 0x00280000>; // 2.5M for kernel
> + };
> + device-tree@3ec0000 {
> + label = "device-tree";
> + reg = <0x03ec0000 0x00040000>; // one sector for device tree
> + };
> + u-boot@3f00000 {
> + label = "u-boot";
> + reg = <0x03f00000 0x00100000>; // 1M for u-boot
> + read-only;
> + };
> };
>
> board-control@2,0 {
> compatible = "fsl,mpc5121ads-cpld";
> reg = <0x2 0x0 0x8000>;
> };
> +
> + cpld_pic: pic@2,a {
> + compatible = "fsl,mpc5121ads-cpld-pic";
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + reg = <0x2 0xa 0x5>;
> + interrupt-parent = < &ipic >;
> + // irq routing
> + // all irqs but touch screen are routed to irq0 (ipic 48)
> + // touch screen is statically routed to irq1 (ipic 17)
> + // so don't use it here
> + interrupts = <48 0x8>;
> + };
> };
>
> soc@80000000 {
> compatible = "fsl,mpc5121-immr";
> + device_type = "soc";
Don't do this. I know it exists on older board ports, but it is bad
practice. Depend entirely on the compatible value instead.
> #address-cells = <1>;
> #size-cells = <1>;
> #interrupt-cells = <2>;
> @@ -85,38 +160,252 @@
> reg = <0xc00 0x100>;
> };
>
> - // 512x PSCs are not 52xx PSCs compatible
> + rtc@a00 { // Real time clock
> + compatible = "fsl,mpc5121-rtc";
> + reg = <0xa00 0x100>;
> + interrupts = <79 0x8 80 0x8>;
> + interrupt-parent = < &ipic >;
> + };
> +
> + clock@f00 { // Clock control
> + compatible = "fsl,mpc5121-clock";
> + reg = <0xf00 0x100>;
> + };
> +
> + pmc@1000{ //Power Management Controller
> + compatible = "fsl,mpc5121-pmc";
> + reg = <0x1000 0x100>;
> + interrupts = <83 0x2>;
> + interrupt-parent = < &ipic >;
> + };
> +
> + gpio@1100 {
> + compatible = "fsl,mpc5121-gpio";
> + reg = <0x1100 0x100>;
> + interrupts = <78 0x8>;
> + interrupt-parent = < &ipic >;
> + };
> +
> + mscan@1300 {
> + compatible = "fsl,mpc5121-mscan";
> + cell-index = <0>;
> + interrupts = <12 0x8>;
> + interrupt-parent = < &ipic >;
> + reg = <0x1300 0x80>;
> + };
> +
> + mscan@1380 {
> + compatible = "fsl,mpc5121-mscan";
> + cell-index = <1>;
> + interrupts = <13 0x8>;
> + interrupt-parent = < &ipic >;
> + reg = <0x1380 0x80>;
> + };
> +
> + i2c@1700 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl-i2c";
should be 'compatible = "fsl,mpc5121-i2c", "fsl-i2c";' for completeness.
Ditto through the rest of the i2c nodes.
> + cell-index = <0>;
> + reg = <0x1700 0x20>;
> + interrupts = <9 0x8>;
> + interrupt-parent = < &ipic >;
> + fsl5200-clocking;
> + };
> +
> + i2c@1720 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl-i2c";
> + cell-index = <1>;
> + reg = <0x1720 0x20>;
> + interrupts = <10 0x8>;
> + interrupt-parent = < &ipic >;
> + fsl5200-clocking;
> + };
> +
> + i2c@1740 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl-i2c";
> + cell-index = <2>;
> + reg = <0x1740 0x20>;
> + interrupts = <11 0x8>;
> + interrupt-parent = < &ipic >;
> + fsl5200-clocking;
> + };
> +
> + i2ccontrol@1760 {
> + compatible = "fsl,mpc5121-i2c-ctrl";
> + reg = <0x1760 0x8>;
> + };
> +
> + axe@2000 {
> + compatible = "fsl,mpc5121-axe";
> + reg = <0x2000 0x100>;
> + interrupts = <42 0x8>;
> + interrupt-parent = < &ipic >;
> + };
> +
> + display@2100 {
> + compatible = "fsl-diu";
should be 'compatible = "fsl,mpc5121-diu", "fsl-diu".
Actually, I don't like "fsl-diu" at all and I'd rather see it gone
entirely, but that is a separate battle.
> + reg = <0x2100 0x100>;
> + interrupts = <64 0x8>;
> + interrupt-parent = < &ipic >;
> + };
> +
> + mdio@2800 {
> + compatible = "fsl,mpc5121-fec-mdio";
> + reg = <0x2800 0x800>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + phy: ethernet-phy@0 {
> + reg = <1>;
> + device_type = "ethernet-phy";
> + };
> + };
> +
> + ethernet@2800 {
> + device_type = "network";
> + compatible = "fsl,mpc5121-fec";
> + reg = <0x2800 0x800>;
> + local-mac-address = [ 00 00 00 00 00 00 ];
> + interrupts = <4 0x8>;
> + interrupt-parent = < &ipic >;
> + phy-handle = < &phy >;
> + fsl,align-tx-packets = <4>;
> + };
> +
> + // 5121e has two dr usb modules
> + // mpc5121_ads only uses USB0
> +
> + // USB1 using external ULPI PHY
> + //usb@3000 {
> + // compatible = "fsl-usb2-dr";
I know it is commented out, but same comment applies.
> + // reg = <0x3000 0x1000>;
> + // #address-cells = <1>;
> + // #size-cells = <0>;
> + // interrupt-parent = < &ipic >;
> + // interrupts = <43 0x8>;
> + // dr_mode = "otg";
> + // phy_type = "ulpi";
> + // port1;
> + //};
> +
> + // USB0 using internal UTMI PHY
> + usb@4000 {
> + compatible = "fsl-usb2-dr";
ditto
> + reg = <0x4000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupt-parent = < &ipic >;
> + interrupts = <44 0x8>;
> + dr_mode = "otg";
> + phy_type = "utmi_wide";
> + port0;
> + };
> +
> + // IO control
> + ioctl@a000 {
> + compatible = "fsl,mpc5121-ioctl";
> + reg = <0xA000 0x1000>;
> + };
> +
> + pata@10200 {
> + compatible = "fsl,mpc5121-pata";
> + reg = <0x10200 0x100>;
> + interrupts = <5 0x8>;
> + interrupt-parent = < &ipic >;
> + };
> +
> + // 512x PSCs are not 52xx PSC compatible
> // PSC3 serial port A aka ttyPSC0
> serial@11300 {
> device_type = "serial";
> - compatible = "fsl,mpc5121-psc-uart";
> + compatible = "fsl,mpc5121-psc-uart", "fsl,mpc5121-psc";
I'm not sure about this, it kind of mixes usages. But on the other
hand, it gracefully solves the problem of identifying all PSCs,
regardless of the mode.... it doesn't break any major conventions, so
yeah; this is probably good.
g.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc: Move mpc83xx_add_bridge to fsl_pci.c [rev2]
From: Grant Likely @ 2008-06-29 6:01 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <1214500077-9254-3-git-send-email-jrigby@freescale.com>
On Thu, Jun 26, 2008 at 11:07:57AM -0600, John Rigby wrote:
> This allows other platforms with the same pci
> block like MPC5121 to use it.
>
> Signed-off-by: John Rigby <jrigby@freescale.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> arch/powerpc/platforms/83xx/Kconfig | 2 +-
> arch/powerpc/platforms/83xx/Makefile | 1 -
> arch/powerpc/platforms/83xx/mpc831x_rdb.c | 1 +
> arch/powerpc/platforms/83xx/mpc832x_mds.c | 1 +
> arch/powerpc/platforms/83xx/mpc832x_rdb.c | 1 +
> arch/powerpc/platforms/83xx/mpc834x_itx.c | 1 +
> arch/powerpc/platforms/83xx/mpc834x_mds.c | 1 +
> arch/powerpc/platforms/83xx/mpc836x_mds.c | 1 +
> arch/powerpc/platforms/83xx/mpc837x_mds.c | 1 +
> arch/powerpc/platforms/83xx/mpc837x_rdb.c | 1 +
> arch/powerpc/platforms/83xx/mpc83xx.h | 1 -
> arch/powerpc/platforms/83xx/pci.c | 91 -----------------------------
> arch/powerpc/platforms/83xx/sbc834x.c | 1 +
> arch/powerpc/sysdev/fsl_pci.c | 61 +++++++++++++++++++
> arch/powerpc/sysdev/fsl_pci.h | 1 +
> 15 files changed, 72 insertions(+), 94 deletions(-)
> delete mode 100644 arch/powerpc/platforms/83xx/pci.c
>
> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/platforms/83xx/Kconfig
> index 0a306eb..ca2c149 100644
> --- a/arch/powerpc/platforms/83xx/Kconfig
> +++ b/arch/powerpc/platforms/83xx/Kconfig
> @@ -3,7 +3,7 @@ menuconfig MPC83xx
> depends on PPC_83xx
> select PPC_UDBG_16550
> select PPC_PCI_CHOICE
> - select PPC_INDIRECT_PCI
> + select FSL_PCI if PCI
>
> if MPC83xx
>
> diff --git a/arch/powerpc/platforms/83xx/Makefile b/arch/powerpc/platforms/83xx/Makefile
> index 76494be..59c413c 100644
> --- a/arch/powerpc/platforms/83xx/Makefile
> +++ b/arch/powerpc/platforms/83xx/Makefile
> @@ -2,7 +2,6 @@
> # Makefile for the PowerPC 83xx linux kernel.
> #
> obj-y := misc.o usb.o
> -obj-$(CONFIG_PCI) += pci.o
> obj-$(CONFIG_MPC831x_RDB) += mpc831x_rdb.o
> obj-$(CONFIG_MPC832x_RDB) += mpc832x_rdb.o
> obj-$(CONFIG_MPC834x_MDS) += mpc834x_mds.o
> diff --git a/arch/powerpc/platforms/83xx/mpc831x_rdb.c b/arch/powerpc/platforms/83xx/mpc831x_rdb.c
> index c4db517..a428f8d 100644
> --- a/arch/powerpc/platforms/83xx/mpc831x_rdb.c
> +++ b/arch/powerpc/platforms/83xx/mpc831x_rdb.c
> @@ -19,6 +19,7 @@
> #include <asm/time.h>
> #include <asm/ipic.h>
> #include <asm/udbg.h>
> +#include <sysdev/fsl_pci.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/platforms/83xx/mpc832x_mds.c b/arch/powerpc/platforms/83xx/mpc832x_mds.c
> index 6dbc6ea..dd4be4a 100644
> --- a/arch/powerpc/platforms/83xx/mpc832x_mds.c
> +++ b/arch/powerpc/platforms/83xx/mpc832x_mds.c
> @@ -36,6 +36,7 @@
> #include <asm/prom.h>
> #include <asm/udbg.h>
> #include <sysdev/fsl_soc.h>
> +#include <sysdev/fsl_pci.h>
> #include <asm/qe.h>
> #include <asm/qe_ic.h>
>
> diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> index e7f706b..f049d69 100644
> --- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> +++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> @@ -27,6 +27,7 @@
> #include <asm/qe.h>
> #include <asm/qe_ic.h>
> #include <sysdev/fsl_soc.h>
> +#include <sysdev/fsl_pci.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/platforms/83xx/mpc834x_itx.c b/arch/powerpc/platforms/83xx/mpc834x_itx.c
> index 50e8f63..7301d77 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
> +++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
> @@ -35,6 +35,7 @@
> #include <asm/prom.h>
> #include <asm/udbg.h>
> #include <sysdev/fsl_soc.h>
> +#include <sysdev/fsl_pci.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/platforms/83xx/mpc834x_mds.c b/arch/powerpc/platforms/83xx/mpc834x_mds.c
> index 2b8a0a3..30d509a 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_mds.c
> +++ b/arch/powerpc/platforms/83xx/mpc834x_mds.c
> @@ -35,6 +35,7 @@
> #include <asm/prom.h>
> #include <asm/udbg.h>
> #include <sysdev/fsl_soc.h>
> +#include <sysdev/fsl_pci.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> index c2e5de6..75b80e8 100644
> --- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
> +++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> @@ -42,6 +42,7 @@
> #include <asm/prom.h>
> #include <asm/udbg.h>
> #include <sysdev/fsl_soc.h>
> +#include <sysdev/fsl_pci.h>
> #include <asm/qe.h>
> #include <asm/qe_ic.h>
>
> diff --git a/arch/powerpc/platforms/83xx/mpc837x_mds.c b/arch/powerpc/platforms/83xx/mpc837x_mds.c
> index 64d17b0..be62de2 100644
> --- a/arch/powerpc/platforms/83xx/mpc837x_mds.c
> +++ b/arch/powerpc/platforms/83xx/mpc837x_mds.c
> @@ -19,6 +19,7 @@
> #include <asm/ipic.h>
> #include <asm/udbg.h>
> #include <asm/prom.h>
> +#include <sysdev/fsl_pci.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/platforms/83xx/mpc837x_rdb.c b/arch/powerpc/platforms/83xx/mpc837x_rdb.c
> index c00356b..da030af 100644
> --- a/arch/powerpc/platforms/83xx/mpc837x_rdb.c
> +++ b/arch/powerpc/platforms/83xx/mpc837x_rdb.c
> @@ -17,6 +17,7 @@
> #include <asm/time.h>
> #include <asm/ipic.h>
> #include <asm/udbg.h>
> +#include <sysdev/fsl_pci.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h b/arch/powerpc/platforms/83xx/mpc83xx.h
> index 88a3b5c..393dfec 100644
> --- a/arch/powerpc/platforms/83xx/mpc83xx.h
> +++ b/arch/powerpc/platforms/83xx/mpc83xx.h
> @@ -55,7 +55,6 @@
> * mpc83xx_* files. Mostly for use by mpc83xx_setup
> */
>
> -extern int mpc83xx_add_bridge(struct device_node *dev);
> extern void mpc83xx_restart(char *cmd);
> extern long mpc83xx_time_init(void);
> extern int mpc834x_usb_cfg(void);
> diff --git a/arch/powerpc/platforms/83xx/pci.c b/arch/powerpc/platforms/83xx/pci.c
> deleted file mode 100644
> index 14f1080..0000000
> --- a/arch/powerpc/platforms/83xx/pci.c
> +++ /dev/null
> @@ -1,91 +0,0 @@
> -/*
> - * FSL SoC setup code
> - *
> - * Maintained by Kumar Gala (see MAINTAINERS for contact information)
> - *
> - * 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.
> - */
> -
> -#include <linux/stddef.h>
> -#include <linux/kernel.h>
> -#include <linux/init.h>
> -#include <linux/errno.h>
> -#include <linux/pci.h>
> -#include <linux/delay.h>
> -#include <linux/irq.h>
> -#include <linux/module.h>
> -
> -#include <asm/system.h>
> -#include <asm/atomic.h>
> -#include <asm/io.h>
> -#include <asm/pci-bridge.h>
> -#include <asm/prom.h>
> -#include <sysdev/fsl_soc.h>
> -
> -#undef DEBUG
> -
> -#ifdef DEBUG
> -#define DBG(x...) printk(x)
> -#else
> -#define DBG(x...)
> -#endif
> -
> -int __init mpc83xx_add_bridge(struct device_node *dev)
> -{
> - int len;
> - struct pci_controller *hose;
> - struct resource rsrc;
> - const int *bus_range;
> - int primary = 1, has_address = 0;
> - phys_addr_t immr = get_immrbase();
> -
> - DBG("Adding PCI host bridge %s\n", dev->full_name);
> -
> - /* Fetch host bridge registers address */
> - has_address = (of_address_to_resource(dev, 0, &rsrc) == 0);
> -
> - /* Get bus range if any */
> - bus_range = of_get_property(dev, "bus-range", &len);
> - if (bus_range == NULL || len < 2 * sizeof(int)) {
> - printk(KERN_WARNING "Can't get bus-range for %s, assume"
> - " bus 0\n", dev->full_name);
> - }
> -
> - ppc_pci_flags |= PPC_PCI_REASSIGN_ALL_BUS;
> - hose = pcibios_alloc_controller(dev);
> - if (!hose)
> - return -ENOMEM;
> -
> - hose->first_busno = bus_range ? bus_range[0] : 0;
> - hose->last_busno = bus_range ? bus_range[1] : 0xff;
> -
> - /* MPC83xx supports up to two host controllers one at 0x8500 from immrbar
> - * the other at 0x8600, we consider the 0x8500 the primary controller
> - */
> - /* PCI 1 */
> - if ((rsrc.start & 0xfffff) == 0x8500) {
> - setup_indirect_pci(hose, immr + 0x8300, immr + 0x8304, 0);
> - }
> - /* PCI 2 */
> - if ((rsrc.start & 0xfffff) == 0x8600) {
> - setup_indirect_pci(hose, immr + 0x8380, immr + 0x8384, 0);
> - primary = 0;
> - }
> -
> - printk(KERN_INFO "Found MPC83xx PCI host bridge at 0x%016llx. "
> - "Firmware bus number: %d->%d\n",
> - (unsigned long long)rsrc.start, hose->first_busno,
> - hose->last_busno);
> -
> - DBG(" ->Hose at 0x%p, cfg_addr=0x%p,cfg_data=0x%p\n",
> - hose, hose->cfg_addr, hose->cfg_data);
> -
> - /* Interpret the "ranges" property */
> - /* This also maps the I/O region and sets isa_io/mem_base */
> - pci_process_bridge_OF_ranges(hose, dev, primary);
> -
> - return 0;
> -}
> diff --git a/arch/powerpc/platforms/83xx/sbc834x.c b/arch/powerpc/platforms/83xx/sbc834x.c
> index cf38247..fc21f5c 100644
> --- a/arch/powerpc/platforms/83xx/sbc834x.c
> +++ b/arch/powerpc/platforms/83xx/sbc834x.c
> @@ -37,6 +37,7 @@
> #include <asm/prom.h>
> #include <asm/udbg.h>
> #include <sysdev/fsl_soc.h>
> +#include <sysdev/fsl_pci.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 489ca5a..68583f6 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -27,6 +27,7 @@
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
>
> +#if defined(CONFIG_PPC_85xx) || defined(CONFIG_PPC_86xx)
> /* atmu setup for fsl pci/pcie controller */
> void __init setup_pci_atmu(struct pci_controller *hose, struct resource *rsrc)
> {
> @@ -246,3 +247,63 @@ DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_MPC8572, quirk_fsl_pcie_header);
> DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_MPC8641, quirk_fsl_pcie_header);
> DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_MPC8641D, quirk_fsl_pcie_header);
> DECLARE_PCI_FIXUP_HEADER(0x1957, PCI_DEVICE_ID_MPC8610, quirk_fsl_pcie_header);
> +#endif /* CONFIG_PPC_85xx || CONFIG_PPC_86xx */
> +
> +#if defined(CONFIG_PPC_83xx)
> +int __init mpc83xx_add_bridge(struct device_node *dev)
> +{
> + int len;
> + struct pci_controller *hose;
> + struct resource rsrc;
> + const int *bus_range;
> + int primary = 1, has_address = 0;
> + phys_addr_t immr = get_immrbase();
> +
> + pr_debug("Adding PCI host bridge %s\n", dev->full_name);
> +
> + /* Fetch host bridge registers address */
> + has_address = (of_address_to_resource(dev, 0, &rsrc) == 0);
> +
> + /* Get bus range if any */
> + bus_range = of_get_property(dev, "bus-range", &len);
> + if (bus_range == NULL || len < 2 * sizeof(int)) {
> + printk(KERN_WARNING "Can't get bus-range for %s, assume"
> + " bus 0\n", dev->full_name);
> + }
> +
> + ppc_pci_flags |= PPC_PCI_REASSIGN_ALL_BUS;
> + hose = pcibios_alloc_controller(dev);
> + if (!hose)
> + return -ENOMEM;
> +
> + hose->first_busno = bus_range ? bus_range[0] : 0;
> + hose->last_busno = bus_range ? bus_range[1] : 0xff;
> +
> + /* MPC83xx supports up to two host controllers one at 0x8500 from immrbar
> + * the other at 0x8600, we consider the 0x8500 the primary controller
> + */
> + /* PCI 1 */
> + if ((rsrc.start & 0xfffff) == 0x8500) {
> + setup_indirect_pci(hose, immr + 0x8300, immr + 0x8304, 0);
> + }
> + /* PCI 2 */
> + if ((rsrc.start & 0xfffff) == 0x8600) {
> + setup_indirect_pci(hose, immr + 0x8380, immr + 0x8384, 0);
> + primary = 0;
> + }
> +
> + printk(KERN_INFO "Found MPC83xx PCI host bridge at 0x%016llx. "
> + "Firmware bus number: %d->%d\n",
> + (unsigned long long)rsrc.start, hose->first_busno,
> + hose->last_busno);
> +
> + pr_debug(" ->Hose at 0x%p, cfg_addr=0x%p,cfg_data=0x%p\n",
> + hose, hose->cfg_addr, hose->cfg_data);
> +
> + /* Interpret the "ranges" property */
> + /* This also maps the I/O region and sets isa_io/mem_base */
> + pci_process_bridge_OF_ranges(hose, dev, primary);
> +
> + return 0;
> +}
> +#endif /* CONFIG_PPC_83xx */
> diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
> index 37b04ad..13f30c2 100644
> --- a/arch/powerpc/sysdev/fsl_pci.h
> +++ b/arch/powerpc/sysdev/fsl_pci.h
> @@ -83,6 +83,7 @@ struct ccsr_pci {
>
> extern int fsl_add_bridge(struct device_node *dev, int is_primary);
> extern void fsl_pcibios_fixup_bus(struct pci_bus *bus);
> +extern int mpc83xx_add_bridge(struct device_node *dev);
>
> #endif /* __POWERPC_FSL_PCI_H */
> #endif /* __KERNEL__ */
> --
> 1.5.6.rc0.46.gd2b3
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: pci config cleanup [rev2]
From: Grant Likely @ 2008-06-29 5:58 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <1214500077-9254-2-git-send-email-jrigby@freescale.com>
On Thu, Jun 26, 2008 at 11:07:56AM -0600, John Rigby wrote:
> Choosing PCI or not at config time is allowed on some
> platforms via an if expression in arch/powerpc/Kconfig.
> To add a new platform with PCI support selectable at
> config time, you must change the if expression. This
> patch makes this easier by changing:
> bool "PCI support" if <long expression>
> to
> bool "PCI support" if PPC_PCI_CHOICE
> and adding select PPC_PCI_CHOICE to all the config nodes that
> were previously in the PCI if expression.
>
> Platforms with unconditional PCI support continue to
> just select PCI in their config nodes.
>
> Signed-off-by: John Rigby <jrigby@freescale.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
^ permalink raw reply
* Re: [PATCH] [RFC] powerpc: Xilinx: adding virtex5 powerpc 440 support
From: Grant Likely @ 2008-06-29 5:52 UTC (permalink / raw)
To: John Linn; +Cc: linuxppc-dev
In-Reply-To: <20080624210750.A21E314006F@mail88-wa4.bigfish.com>
On Tue, Jun 24, 2008 at 03:07:48PM -0600, John Linn wrote:
> Hi Grant,
>
> It appears that you designed the simpleImage around Xilinx FPGAs.
Yes.
> Since we have to initialize the 16550 UART in the bootstrap and there's
> no boot loader, were you thinking we would add the 16550 initialization
> to simpleboot.c?
Yes; or at least break out the common routine to be called by all
simpleImage targets and have a virtex-specific 'frontend' to it called
simpleboot-virtex405.c.
>
> Why do we want a flat binary rather than an elf file for the target?
'Cause a flat binary can be loaded anywhere in RAM and is smaller than
an ELF image (no headers).
g.
^ permalink raw reply
* Re: [PATCH] [RFC] powerpc: Xilinx: adding virtex5 powerpc 440 support
From: Grant Likely @ 2008-06-29 5:50 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, John Linn
In-Reply-To: <20080623213556.3378316a@zod.rchland.ibm.com>
On Mon, Jun 23, 2008 at 09:35:56PM -0400, Josh Boyer wrote:
> On Mon, 23 Jun 2008 15:30:35 -0600 John Linn <John.Linn@xilinx.com> wrote:
> > I'll try to better understand if we can detect the compressed device
> > tree and if we really have to disable the APU.
> >
> > What's the reasoning for being independent of the kernel, maybe it's
> > obvious to everyone but me?
>
> The intention, as I understand it, is that the wrapper utilities can be
> installed stand-alone and used to wrap other kernels if needs be. In
> practice I've not seen this happen yet, as most PowerPC kernels
> are built directly from the kernel source. Fedora does have a
> separate package for the wrapper bits, but I'm not entirely sure it's
> used.
>
> My understanding could be totally wrong, and if so I'll politely ask
> Paul or anyone else to hit me with a cluebat :).
AFAIK, the reason is to be able to build multiple bootwrapper
configurations around a single kernel image without multiple compiles of
the wrapper bits, and to increase the amount of compile testing that all
the wrapper bits are subjected to. (this way they get compiled on all
PowerPC kernel compiles instead of just when they are needed for an
obscure platform)
g.
^ permalink raw reply
* Re: [PATCH 6/6] MPC5121 Hide pci bridge
From: Grant Likely @ 2008-06-29 5:42 UTC (permalink / raw)
To: John Rigby, galak; +Cc: linuxppc-dev
In-Reply-To: <1213981119-1979-7-git-send-email-jrigby@freescale.com>
On Fri, Jun 20, 2008 at 10:58:39AM -0600, John Rigby wrote:
> The class of the MPC5121 pci host bridge is PCI_CLASS_BRIDGE_OTHER
> while other freescale host bridges have class set to
> PCI_CLASS_PROCESSOR_POWERPC.
>
> This patch makes fixup_hide_host_resource_fsl match
> PCI_CLASS_BRIDGE_OTHER in addition to PCI_CLASS_PROCESSOR_POWERPC.
>
> Signed-off-by: John Rigby <jrigby@freescale.com>
This looks right to me, but I'm not really qualified to say if it is
correct or not (limited PCI experience). I don't know who needs to look
at it. Kumar perhaps?
Cheers,
g.
> ---
> arch/powerpc/kernel/pci_32.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
> index 88db4ff..3d33935 100644
> --- a/arch/powerpc/kernel/pci_32.c
> +++ b/arch/powerpc/kernel/pci_32.c
> @@ -54,11 +54,12 @@ LIST_HEAD(hose_list);
> static int pci_bus_count;
>
> static void
> -fixup_hide_host_resource_fsl(struct pci_dev* dev)
> +fixup_hide_host_resource_fsl(struct pci_dev *dev)
> {
> int i, class = dev->class >> 8;
>
> - if ((class == PCI_CLASS_PROCESSOR_POWERPC) &&
> + if ((class == PCI_CLASS_PROCESSOR_POWERPC
> + || class == PCI_CLASS_BRIDGE_OTHER) &&
> (dev->hdr_type == PCI_HEADER_TYPE_NORMAL) &&
> (dev->bus->parent == NULL)) {
> for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> --
> 1.5.6.rc0.46.gd2b3
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 5/6] MPC5121 Add PCI support
From: Grant Likely @ 2008-06-29 5:38 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <1213981119-1979-6-git-send-email-jrigby@freescale.com>
On Fri, Jun 20, 2008 at 10:58:38AM -0600, John Rigby wrote:
> Copied from 83xx minus support for two busses.
If this is a copy, then can it be shared?
g.
^ permalink raw reply
* Re: [PATCH 4/6] MPC5121 Add MPC5121ADS cpld support
From: Grant Likely @ 2008-06-29 5:36 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <1213981119-1979-5-git-send-email-jrigby@freescale.com>
Minor comments below.
On Fri, Jun 20, 2008 at 10:58:37AM -0600, John Rigby wrote:
> Add a interrupt host for the interrupt
> controller in the mpc5121ads cpld.
> PCI interrupts are 0-7 the rest are 8-15
> Touchscreen pendown irq is hardwired to irq1
> All other irqs are chainged to irq0
>
> Signed-off-by: John Rigby <jrigby@freescale.com>
> ---
> arch/powerpc/platforms/512x/Kconfig | 1 +
> arch/powerpc/platforms/512x/Makefile | 2 +-
> arch/powerpc/platforms/512x/mpc5121_ads.c | 14 ++-
> arch/powerpc/platforms/512x/mpc5121_ads.h | 14 ++
> arch/powerpc/platforms/512x/mpc5121_ads_cpld.c | 204 ++++++++++++++++++++++++
> 5 files changed, 233 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/platforms/512x/mpc5121_ads.h
> create mode 100644 arch/powerpc/platforms/512x/mpc5121_ads_cpld.c
>
> diff --git a/arch/powerpc/platforms/512x/Kconfig b/arch/powerpc/platforms/512x/Kconfig
> index f9a04da..0fd3b00 100644
> --- a/arch/powerpc/platforms/512x/Kconfig
> +++ b/arch/powerpc/platforms/512x/Kconfig
> @@ -12,6 +12,7 @@ config MPC5121_ADS
> depends on PPC_MULTIPLATFORM && PPC32
> select DEFAULT_UIMAGE
> select PPC_MPC5121
> + select MPC5121_ADS_CPLD
What is this for? I don't see it used anywhere.
> help
> This option enables support for the MPC5121E ADS board.
>
> diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_ads.c
> index 45bb2ef..36805fd 100644
> --- a/arch/powerpc/platforms/512x/mpc5121_ads.c
> +++ b/arch/powerpc/platforms/512x/mpc5121_ads.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (C) 2007 Freescale Semiconductor, Inc. All rights reserved.
> + * Copyright (C) 2007, 2008 Freescale Semiconductor, Inc. All rights reserved.
> *
> * Author: John Rigby, <jrigby@freescale.com>, Thur Mar 29 2007
> *
> @@ -23,6 +23,16 @@
> #include <asm/time.h>
>
> #include "mpc512x.h"
> +#include "mpc5121_ads.h"
> +
> +static void __init mpc5121_ads_setup_arch(void)
> +{
> + printk(KERN_INFO "MPC5121 ADS board from Freescale Semiconductor\n");
> + /*
> + * cpld regs are needed early
> + */
> + mpc5121_ads_cpld_map();
> +}
>
> static struct of_device_id __initdata of_bus_ids[] = {
> { .name = "soc", },
> @@ -41,6 +51,7 @@ static void __init mpc5121_ads_declare_of_platform_devices(void)
> static void __init mpc5121_ads_init_IRQ(void)
> {
> mpc512x_init_IRQ();
> + mpc5121_ads_cpld_pic_init();
Ah, I understand now. Ignore my related comment in the previous patch.
> }
>
> /*
^ permalink raw reply
* Re: [PATCH 3/6] MPC5121 Add generic board support
From: Grant Likely @ 2008-06-29 5:30 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <1213981119-1979-4-git-send-email-jrigby@freescale.com>
Mostly looks good, but a few comments below.
On Fri, Jun 20, 2008 at 10:58:36AM -0600, John Rigby wrote:
> Move shared code from mpc5121_ads.c to mpc512x_shared.c.
> Add new generic board setup mpc5121_generic.c
>
> Signed-off-by: John Rigby <jrigby@freescale.com>
> ---
> arch/powerpc/platforms/512x/Kconfig | 15 ++++-
> arch/powerpc/platforms/512x/Makefile | 3 +-
> arch/powerpc/platforms/512x/mpc5121_ads.c | 45 +---------------
> arch/powerpc/platforms/512x/mpc5121_generic.c | 72 +++++++++++++++++++++++++
> arch/powerpc/platforms/512x/mpc512x.h | 14 +++++
> arch/powerpc/platforms/512x/mpc512x_shared.c | 66 ++++++++++++++++++++++
> 6 files changed, 168 insertions(+), 47 deletions(-)
> create mode 100644 arch/powerpc/platforms/512x/mpc5121_generic.c
> create mode 100644 arch/powerpc/platforms/512x/mpc512x.h
> create mode 100644 arch/powerpc/platforms/512x/mpc512x_shared.c
>
> diff --git a/arch/powerpc/platforms/512x/Kconfig b/arch/powerpc/platforms/512x/Kconfig
> index 4c0da0c..f9a04da 100644
> --- a/arch/powerpc/platforms/512x/Kconfig
> +++ b/arch/powerpc/platforms/512x/Kconfig
> @@ -2,12 +2,10 @@ config PPC_MPC512x
> bool
> select FSL_SOC
> select IPIC
> - default n
>
> config PPC_MPC5121
> bool
> select PPC_MPC512x
> - default n
>
> config MPC5121_ADS
> bool "Freescale MPC5121E ADS"
> @@ -16,4 +14,15 @@ config MPC5121_ADS
> select PPC_MPC5121
> help
> This option enables support for the MPC5121E ADS board.
> - default n
> +
> +config MPC5121_GENERIC
> + bool "Generic support for simple MPC5121 based boards"
> + depends on PPC_MULTIPLATFORM && PPC32
> + select DEFAULT_UIMAGE
> + select PPC_MPC5121
> + help
> + This option enables support for simple MPC5121 based boards
> + which do not need custome platform specific setup.
typo
> +
> + Compatible boards include: Protonic LVT base boards (ZANMCU
> + and VICVT2).
> diff --git a/arch/powerpc/platforms/512x/Makefile b/arch/powerpc/platforms/512x/Makefile
> index ef6c925..e6674c8 100644
> --- a/arch/powerpc/platforms/512x/Makefile
> +++ b/arch/powerpc/platforms/512x/Makefile
> @@ -1,5 +1,6 @@
> #
> # Makefile for the Freescale PowerPC 512x linux kernel.
> #
> -obj-y := clock.o
> +obj-y := clock.o mpc512x_shared.o
> obj-$(CONFIG_MPC5121_ADS) += mpc5121_ads.o
> +obj-$(CONFIG_MPC5121_GENERIC) += mpc5121_generic.o
> diff --git a/arch/powerpc/platforms/512x/mpc5121_ads.c b/arch/powerpc/platforms/512x/mpc5121_ads.c
> index 50bd3a3..45bb2ef 100644
> --- a/arch/powerpc/platforms/512x/mpc5121_ads.c
> +++ b/arch/powerpc/platforms/512x/mpc5121_ads.c
> @@ -68,20 +40,7 @@ static void __init mpc5121_ads_declare_of_platform_devices(void)
>
> static void __init mpc5121_ads_init_IRQ(void)
> {
> - struct device_node *np;
> -
> - np = of_find_compatible_node(NULL, NULL, "fsl,ipic");
> - if (!np)
> - return;
> -
> - ipic_init(np, 0);
> - of_node_put(np);
> -
> - /*
> - * Initialize the default interrupt mapping priorities,
> - * in case the boot rom changed something on us.
> - */
> - ipic_set_default_priority();
> + mpc512x_init_IRQ();
> }
Why not just put mpc512x_init_IRQ directly into the machine structure?
>
> /*
> diff --git a/arch/powerpc/platforms/512x/mpc5121_generic.c b/arch/powerpc/platforms/512x/mpc5121_generic.c
> new file mode 100644
> index 0000000..0111a98
> --- /dev/null
> +++ b/arch/powerpc/platforms/512x/mpc5121_generic.c
> @@ -0,0 +1,72 @@
> +/*
> + * Copyright (C) 2007,2008 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * Author: John Rigby, <jrigby@freescale.com>
> + *
> + * Description:
> + * MPC5121 SoC setup
> + *
> + * This 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.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/machdep.h>
> +#include <asm/ipic.h>
> +#include <asm/prom.h>
> +#include <asm/time.h>
> +
> +#include "mpc512x.h"
> +
> +static struct of_device_id __initdata of_bus_ids[] = {
> + { .name = "soc", },
> + { .name = "localbus", },
Ugh. Not good. Bind against compatible, not name or type.
> + {},
> +};
> +
> +static void __init mpc5121_generic_declare_of_platform_devices(void)
> +{
> + /* Find every child of the SOC node and add it to of_platform */
> + if (of_platform_bus_probe(NULL, of_bus_ids, NULL))
> + printk(KERN_ERR __FILE__ ": "
> + "Error while probing of_platform bus\n");
> +}
> +
> +/*
> + * list of supported boards
> + */
> +static char *board[] __initdata = {
> + "prt,prtlvt",
> + NULL
> +};
> +
> +/*
> + * Called very early, MMU is off, device-tree isn't unflattened
> + */
> +static int __init mpc5121_generic_probe(void)
> +{
> + unsigned long node = of_get_flat_dt_root();
> + int i = 0;
> +
> + while (board[i]) {
> + if (of_flat_dt_is_compatible(node, board[i]))
> + break;
> + i++;
> + }
> +
> + return board[i] != NULL;
> +}
> +
> +define_machine(mpc5121_generic) {
> + .name = "MPC5121 generic",
> + .probe = mpc5121_generic_probe,
> + .init = mpc5121_generic_declare_of_platform_devices,
> + .init_IRQ = mpc512x_init_IRQ,
> + .get_irq = ipic_get_irq,
> + .calibrate_decr = generic_calibrate_decr,
> +};
> diff --git a/arch/powerpc/platforms/512x/mpc512x.h b/arch/powerpc/platforms/512x/mpc512x.h
> new file mode 100644
> index 0000000..789b817
> --- /dev/null
> +++ b/arch/powerpc/platforms/512x/mpc512x.h
> @@ -0,0 +1,14 @@
> +/*
> + * Copyright (C) 2007 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * 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.
> + */
Should probably state what this file is for in the header block.
> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
> new file mode 100644
> index 0000000..4b8fe6a
> --- /dev/null
> +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (C) 2007,2008 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * Author: John Rigby <jrigby@freescale.com>
> + *
> + * Description:
> + * MPC512x Shared code
> + *
> + * This 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/machdep.h>
> +#include <asm/ipic.h>
> +#include <asm/prom.h>
> +#include <asm/time.h>
> +
> +#include "mpc512x.h"
> +
> +unsigned long
> +mpc512x_find_ips_freq(struct device_node *node)
> +{
> + struct device_node *np;
> + const unsigned int *p_ips_freq = NULL;
> +
> + of_node_get(node);
> + while (node) {
> + p_ips_freq = of_get_property(node, "bus-frequency", NULL);
> + if (p_ips_freq)
> + break;
> +
> + np = of_get_parent(node);
> + of_node_put(node);
> + node = np;
> + }
> + if (node)
> + of_node_put(node);
> +
> + return p_ips_freq ? *p_ips_freq : 0;
> +}
> +EXPORT_SYMBOL(mpc512x_find_ips_freq);
This looks identical to the 52xx version. Perhaps they should be
merged... I wouldn't bother for getting this mainlined, but it is
something to think about for the future.
g.
^ permalink raw reply
* Re: [PATCH 2/6] MPC5121 clock driver
From: Grant Likely @ 2008-06-29 5:21 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <1213981119-1979-3-git-send-email-jrigby@freescale.com>
Mostly looks good, a few comments below.
On Fri, Jun 20, 2008 at 10:58:35AM -0600, John Rigby wrote:
> Implements the api defined in include/clk.h
>
> Current only getting frequencies is supported
> not setting.
Need a more detailed commit message. This doesn't tell me much.
>
> Signed-off-by: John Rigby <jrigby@freescale.com>
> ---
> arch/powerpc/platforms/512x/Makefile | 1 +
> arch/powerpc/platforms/512x/clock.c | 729 ++++++++++++++++++++++++++++++++++
> 2 files changed, 730 insertions(+), 0 deletions(-)
> create mode 100644 arch/powerpc/platforms/512x/clock.c
>
> diff --git a/arch/powerpc/platforms/512x/Makefile b/arch/powerpc/platforms/512x/Makefile
> index 232c89f..ef6c925 100644
> --- a/arch/powerpc/platforms/512x/Makefile
> +++ b/arch/powerpc/platforms/512x/Makefile
> @@ -1,4 +1,5 @@
> #
> # Makefile for the Freescale PowerPC 512x linux kernel.
> #
> +obj-y := clock.o
should be +=
> obj-$(CONFIG_MPC5121_ADS) += mpc5121_ads.o
> diff --git a/arch/powerpc/platforms/512x/clock.c b/arch/powerpc/platforms/512x/clock.c
> new file mode 100644
> index 0000000..39db722
> --- /dev/null
> +++ b/arch/powerpc/platforms/512x/clock.c
> @@ -0,0 +1,729 @@
> +/*
> + * Copyright (C) 2007,2008 Freescale Semiconductor, Inc. All rights reserved.
> + *
> + * Author: John Rigby <jrigby@freescale.com>
> + *
> + * Implements the clk api defined in include/linux/clk.h
> + *
> + * Original based on linux/arch/arm/mach-integrator/clock.c
> + *
> + * Copyright (C) 2004 ARM Limited.
> + * Written by Deep Blue Solutions Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/string.h>
> +#include <linux/clk.h>
> +#include <linux/mutex.h>
> +#include <linux/io.h>
> +
> +#include <linux/of_platform.h>
> +#include <asm/mpc512x.h>
> +
> +static int clocks_initialized;
> +
> +struct module;
This is already defined in linux/module.h?
> +
> +#undef CLK_DEBUG
I think this line should be at the top of the file to be easier to find
when toggling it.
> +#ifdef CLK_DEBUG
> +void dump_clocks(void)
> +{
> + struct clk *p;
> +
> + mutex_lock(&clocks_mutex);
> + printk(KERN_INFO "CLOCKS:\n");
> + list_for_each_entry(p, &clocks, node) {
> + printk(KERN_INFO " %s %ld", p->name, p->rate);
> + if (p->parent)
> + printk(KERN_INFO " %s %ld", p->parent->name,
> + p->parent->rate);
> + if (p->flags & CLK_HAS_CTRL)
> + printk(KERN_INFO " reg/bit %d/%d", p->reg, p->bit);
> + printk("\n");
> + }
> + mutex_unlock(&clocks_mutex);
> +}
> +#define DEBUG_CLK_DUMP() dump_clocks()
> +#else
> +#define DEBUG_CLK_DUMP()
> +#endif
> +
> +static long ips_to_ref(unsigned long rate)
> +{
> + int ips_div = (clockctl->scfr1 >> 23) & 0x7;
> +
> + rate *= ips_div; /* csb_clk = ips_clk * ips_div */
> + rate *= 2; /* sys_clk = csb_clk * 2 */
> + return sys_to_ref(rate);
> +}
> +
> +static unsigned long devtree_getfreq(char *nodetype, char *clockname)
Why is nodetype even passed in here? It isn't used, and besides, you
shouldn't test against device_type anyway. Test against compatible
instead.
> +{
> + struct device_node *node;
> + const unsigned int *fp;
> + unsigned int val = 0;
> +
> + node = of_find_node_by_type(NULL, "soc");
Once again; don't look for device_type == "soc". Use compatible.
> + if (node) {
> + fp = of_get_property(node, clockname, NULL);
> + if (fp)
> + val = of_read_ulong(fp, 1);
> + }
> + return val;
> +}
> +
> +static void ref_clk_calc(struct clk *clk)
> +{
> + unsigned long rate;
> +
> + rate = devtree_getfreq("soc", "ref-frequency");
> + if (rate == 0) {
> + /*
> + * no reference clock in device tree
> + * get ips clock freq and go backwards from there
> + */
> + rate = devtree_getfreq("soc", "bus-frequency");
> + if (rate == 0) {
> + printk(KERN_WARNING
> + "No bus-frequency in dev tree using 66MHz\n");
> + clk->rate = 66000000;
Is it even worth trying to use a default here? I think it should fail
loudly instead to reduce the risk of people shipping boards with badly
formed device trees. I don't think there is any backwards compatibility
need for doing this.
^ permalink raw reply
* Re: [PATCH] Convert i2c-mpc from a platform driver to an of_platform one
From: Grant Likely @ 2008-06-29 4:57 UTC (permalink / raw)
To: Jon Smirl; +Cc: linuxppc-dev list, Linux I2C
In-Reply-To: <9e4733910806101940o7f2f9863jb5e556ee2fc39a7e@mail.gmail.com>
On Tue, Jun 10, 2008 at 8:40 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> Convert i2c-mpc from a platform driver into an of_platform driver.
> This patch is much smaller since Jochen already added
> of_find_i2c_driver(). Versions of this have been posted before.
>
> Signed-ff-by: Jon Smirl <jonsmirl@gmail.com>
>
> --
> + if (of_device_is_compatible(op->node, "mpc5200-i2c"))
> + i2c->flags |= FSL_I2C_DEV_CLOCK_5200;
This needs to also test for "fsl,mpc5200-i2c". "mpc5200-i2c" isn't
used on current device trees, but there may be some deployed boards
which still have this string.
> +static const struct of_device_id mpc_i2c_of_match[] = {
> + {
> + .compatible = "fsl-i2c",
> + },
You can probably shorten this by 2 lines if you change it to:
+ { .compatible = "fsl-i2c", },
Doing so makes it more readable when you need to add additional
compatible values.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [i2c] [PATCH] Convert i2c-mpc from a platform driver to an of_platform one
From: Grant Likely @ 2008-06-29 4:49 UTC (permalink / raw)
To: Jon Smirl
Cc: Wolfram Sang, linuxppc-dev list, Linux I2C, Jean Delvare,
Timur Tabi
In-Reply-To: <9e4733910806281905l61714e33h8b26870e2c93539e@mail.gmail.com>
On Sat, Jun 28, 2008 at 10:05:28PM -0400, Jon Smirl wrote:
> On 6/25/08, Jean Delvare <khali@linux-fr.org> wrote:
> > >
> > > i2c->adap = mpc_ops;
> > > - i2c->adap.nr = pdev->id;
> > > i2c_set_adapdata(&i2c->adap, i2c);
> > > - i2c->adap.dev.parent = &pdev->dev;
> > > - if ((result = i2c_add_numbered_adapter(&i2c->adap)) < 0) {
> > > + i2c->adap.dev.parent = &op->dev;
> > > +
> > > + result = i2c_add_adapter(&i2c->adap);
> >
> >
> > The driver was previously using i2c_add_numbered_adapter(), giving MPC
> > platform the possibility to use new-style i2c drivers:
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=1469fa263870acd890a4b9f6ef557acc5d673b44
> > You're breaking this, I doubt it's on purpose?
>
> Grant, what do you want here? You're the one who converted it to
> i2c_add_numbered_adapter. But in other posts you've said that the
> device tree should have nothing to do with bus numbering.
Yes, I did make that change, but that was when it was a platform bus
driver. Converting it to an of_platform bus driver entirely changes the
situation and it should go back to using i2c_add_adapter() with a parse
of the device tree for child nodes.
> Once this driver is converted to an OF one it shouldn't need bus ids
> since all of the i2c devices will be children of the bus node. We can
> just let the i2c subsystem assign a bus number.
Exactly.
> Timur has some issues with the i2c bus number in his ALSA driver. The
> problem is locating the i2c device when the i2s driver loads. Parsing
> the device tree to extract an i2c bus and device number is not a good
> solution.
The trick here is to store the pointer to the device node inside the
i2c device. I do this with SPI devices like this:
/* Store a pointer to the node in the device structure */
of_node_get(nc);
spi->dev.archdata.of_node = nc;
Then, when you've got a device_node pointer, you can parse through the
set of registered SPI devices and match against the one that has the
same device node pointer.
> codec-handle should give you the i2c device node. But then we can't
> use of_find_device_by_node because the i2c device is not an of_device,
> it's a cross platform i2c_device. Should of_find_device_by_node()
> return a 'struct device' instead of a 'struct of_device' and leave it
> as a user exercise to cast up? It is used nine times in the kernel,
> mostly sparc.
No, this doesn't work because I2C and SPI devices are not of_platform
devices. They aren't even platform devices. of_find_device_by_node()
only works for the of_platform bus.
Using archdata.of_node is part of the device structure and works for
every bus type (platform, of_platform, SPI, I2C, PCI, etc.)
g.
^ permalink raw reply
* Re: [RFC] Non-numbered ibm iic driver
From: Grant Likely @ 2008-06-29 4:31 UTC (permalink / raw)
To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <20080628234339.382f31ac@lappy.seanm.ca>
On Sat, Jun 28, 2008 at 11:43:39PM -0400, Sean MacLennan wrote:
> On Sat, 28 Jun 2008 23:25:05 -0400
> "Jon Smirl" <jonsmirl@gmail.com> wrote:
>
> > On 6/28/08, Sean MacLennan <smaclennan@pikatech.com> wrote:
> > > This is a patch to the ibm iic driver that uses the non-numbered
> > > i2c call and therefore does not need an index. Instead, it
> > > registers the ibm iic, then walks all the child nodes and adds
> > > them. This is required for new style drivers, old style drivers
> > > "just work".
> >
> > Check out the code in drivers/of/of_i2c.c. Can you use it instead?
>
> Sure can. The for loop can be replaced with:
>
> of_register_i2c_devices(adap, np);
>
> I have tested it and it works. I guess it makes sense to put of_i2c.c
> under drivers/of, but if it had been under drivers/i2c, I would have
> noticed it ;)
>
> But is this the way we want to go? I personally like it. It gets rid of
> the index and gets rid of the i2c_register_board_info() from the
> platform code.
Oops, forgot to include the list on my first reply...
Yes, this is the way to go. I've got a patch that does the same thing
for SPI busses which I need to post for 2nd review.
g.
^ permalink raw reply
* Re: [RFC] Non-numbered ibm iic driver
From: Sean MacLennan @ 2008-06-29 3:43 UTC (permalink / raw)
To: Jon Smirl; +Cc: linuxppc-dev
In-Reply-To: <9e4733910806282025k6d9415c1g89ff47b739996159@mail.gmail.com>
On Sat, 28 Jun 2008 23:25:05 -0400
"Jon Smirl" <jonsmirl@gmail.com> wrote:
> On 6/28/08, Sean MacLennan <smaclennan@pikatech.com> wrote:
> > This is a patch to the ibm iic driver that uses the non-numbered
> > i2c call and therefore does not need an index. Instead, it
> > registers the ibm iic, then walks all the child nodes and adds
> > them. This is required for new style drivers, old style drivers
> > "just work".
>
> Check out the code in drivers/of/of_i2c.c. Can you use it instead?
Sure can. The for loop can be replaced with:
of_register_i2c_devices(adap, np);
I have tested it and it works. I guess it makes sense to put of_i2c.c
under drivers/of, but if it had been under drivers/i2c, I would have
noticed it ;)
But is this the way we want to go? I personally like it. It gets rid of
the index and gets rid of the i2c_register_board_info() from the
platform code.
Cheers,
Sean
^ permalink raw reply
* Re: [RFC] Non-numbered ibm iic driver
From: Jon Smirl @ 2008-06-29 3:25 UTC (permalink / raw)
To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <20080628232010.2bff2dcc@lappy.seanm.ca>
On 6/28/08, Sean MacLennan <smaclennan@pikatech.com> wrote:
> This is a patch to the ibm iic driver that uses the non-numbered
> i2c call and therefore does not need an index. Instead, it registers the
> ibm iic, then walks all the child nodes and adds them. This is required
> for new style drivers, old style drivers "just work".
Check out the code in drivers/of/of_i2c.c. Can you use it instead?
>
> The warp has both a new style driver (ad7414) and old style (eeprom)
> devices.
>
> This patch is completely function but not a complete patch (the index
> code is not needed for example). It is just for comment.
>
> The warp.dts needed for this to work is, I believe, in Josh's next tree.
>
> Cheers,
> Sean
>
> P.S. Do I need a signed-off-by for an RFC?
>
> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
> ---
>
> diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
> index 85dbf34..0ec6849 100644
> --- a/drivers/i2c/busses/i2c-ibm_iic.c
> +++ b/drivers/i2c/busses/i2c-ibm_iic.c
> @@ -753,7 +753,7 @@ static int __devinit iic_probe(struct ocp_device *ocp){
> */
> adap->nr = dev->idx >= 0 ? dev->idx : 0;
>
> - if ((ret = i2c_add_numbered_adapter(adap)) < 0) {
> + if ((ret = i2c_add_adapter(adap)) < 0) {
> printk(KERN_ERR "ibm-iic%d: failed to register i2c adapter\n",
> dev->idx);
> goto fail;
> @@ -874,6 +874,7 @@ static int __devinit iic_probe(struct of_device *ofdev,
> const struct of_device_id *match)
> {
> struct device_node *np = ofdev->node;
> + struct device_node *child;
> struct ibm_iic_private *dev;
> struct i2c_adapter *adap;
> const u32 *indexp, *freq;
> @@ -939,12 +940,33 @@ static int __devinit iic_probe(struct of_device *ofdev,
> adap->timeout = 1;
> adap->nr = dev->idx;
>
> - ret = i2c_add_numbered_adapter(adap);
> + ret = i2c_add_adapter(adap);
> if (ret < 0) {
> dev_err(&ofdev->dev, "failed to register i2c adapter\n");
> goto error_cleanup;
> }
>
> + for_each_child_of_node(np, child) {
> + struct i2c_board_info info;
> + const u32 *reg;
> +
> + reg = of_get_property(child, "reg", NULL);
> + if (!reg) {
> + printk(KERN_ERR "Could not find address for %s\n",
> + child->name);
> + continue;
> + }
> +
> + memset(&info, 0, sizeof(info));
> + strlcpy(info.type, child->name, I2C_NAME_SIZE);
> + info.addr = *reg;
> +
> + if (!i2c_new_device(adap, &info))
> + printk(KERN_ERR "Could not add i2c device %s.\n",
> + child->name);
> + }
> +
> +
> dev_info(&ofdev->dev, "using %s mode\n",
> dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* [RFC] Non-numbered ibm iic driver
From: Sean MacLennan @ 2008-06-29 3:20 UTC (permalink / raw)
To: linuxppc-dev
This is a patch to the ibm iic driver that uses the non-numbered
i2c call and therefore does not need an index. Instead, it registers the
ibm iic, then walks all the child nodes and adds them. This is required
for new style drivers, old style drivers "just work".
The warp has both a new style driver (ad7414) and old style (eeprom)
devices.
This patch is completely function but not a complete patch (the index
code is not needed for example). It is just for comment.
The warp.dts needed for this to work is, I believe, in Josh's next tree.
Cheers,
Sean
P.S. Do I need a signed-off-by for an RFC?
Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 85dbf34..0ec6849 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -753,7 +753,7 @@ static int __devinit iic_probe(struct ocp_device *ocp){
*/
adap->nr = dev->idx >= 0 ? dev->idx : 0;
- if ((ret = i2c_add_numbered_adapter(adap)) < 0) {
+ if ((ret = i2c_add_adapter(adap)) < 0) {
printk(KERN_ERR "ibm-iic%d: failed to register i2c adapter\n",
dev->idx);
goto fail;
@@ -874,6 +874,7 @@ static int __devinit iic_probe(struct of_device *ofdev,
const struct of_device_id *match)
{
struct device_node *np = ofdev->node;
+ struct device_node *child;
struct ibm_iic_private *dev;
struct i2c_adapter *adap;
const u32 *indexp, *freq;
@@ -939,12 +940,33 @@ static int __devinit iic_probe(struct of_device *ofdev,
adap->timeout = 1;
adap->nr = dev->idx;
- ret = i2c_add_numbered_adapter(adap);
+ ret = i2c_add_adapter(adap);
if (ret < 0) {
dev_err(&ofdev->dev, "failed to register i2c adapter\n");
goto error_cleanup;
}
+ for_each_child_of_node(np, child) {
+ struct i2c_board_info info;
+ const u32 *reg;
+
+ reg = of_get_property(child, "reg", NULL);
+ if (!reg) {
+ printk(KERN_ERR "Could not find address for %s\n",
+ child->name);
+ continue;
+ }
+
+ memset(&info, 0, sizeof(info));
+ strlcpy(info.type, child->name, I2C_NAME_SIZE);
+ info.addr = *reg;
+
+ if (!i2c_new_device(adap, &info))
+ printk(KERN_ERR "Could not add i2c device %s.\n",
+ child->name);
+ }
+
+
dev_info(&ofdev->dev, "using %s mode\n",
dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox