| Message ID | 1238739044-19926-2-git-send-email-qingtao.cao@windriver.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Fri, 2009-04-03 at 14:10 +0800, Harry Ciao wrote: > Setup a platform device for the CPC925 Memory Controller during system > booting up, against which CPC925 MC EDAC driver would be matched. > > Signed-off-by: Harry Ciao <qingtao.cao@windriver.com> > --- > arch/powerpc/platforms/maple/setup.c | 47 ++++++++++++++++++++++++++++++++++ > 1 files changed, 47 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/platforms/maple/setup.c b/arch/powerpc/platforms/maple/setup.c > index bfd60e4..ca8a3ff 100644 > --- a/arch/powerpc/platforms/maple/setup.c > +++ b/arch/powerpc/platforms/maple/setup.c > @@ -335,3 +335,50 @@ define_machine(maple) { > .progress = maple_progress, > .power_save = power4_idle, > }; > + > +#ifdef CONFIG_EDAC > +#define CPC925_MC_START 0xf8000000 > +#define CPC925_MC_END 0xf8ffffff /* sizeof 16MB */ > +/* Register a platform device for CPC925 memory controller */ > +static int __init maple_cpc925_edac_setup(void) > +{ > + struct platform_device *pdev; > + struct device_node *np = NULL; > + struct resource r; > + > + np = of_find_node_by_name(NULL, "hostbridge"); This is way too generic IMHO. You're going to match on any node called "hostbridge" on any 64-bit powerpc machine, there's a good chance there are other firmwares out there with such a node, or will be in the future. Is there no compatible or model property you can look for, so you're at least somewhat confident you're actually looking at a CPC925? If not this should be a machine_initcall() for Maple only. > + if (of_address_to_resource(np, 0, &r)) { > + /* > + * of_address_to_resource() would get the #address-cells > + * and #size-cells properties for a node from its parent. > + * On Maple 64bit target hostbridge parent node has specified > + * both of these two properties to be 2. However, the actual > + * "cell" value for host bridge node is 1. Since we can't > + * fix this firmware-generated DTB, we have to setup a > + * resource structure manually. > + */ > + memset(&r, 0, sizeof(r)); > + r.start = CPC925_MC_START; > + r.end = CPC925_MC_END; > + r.name = "hostbridge"; > + r.flags = IORESOURCE_MEM; We do have fixups for bogus firmware including maple, look in prom_init.c:fixup_device_tree_maple(). Although that's for actual firmware, not a DTB. cheers
Michael Ellerman 写道: > On Fri, 2009-04-03 at 14:10 +0800, Harry Ciao wrote: > >> Setup a platform device for the CPC925 Memory Controller during system >> booting up, against which CPC925 MC EDAC driver would be matched. >> >> Signed-off-by: Harry Ciao <qingtao.cao@windriver.com> >> --- >> arch/powerpc/platforms/maple/setup.c | 47 ++++++++++++++++++++++++++++++++++ >> 1 files changed, 47 insertions(+), 0 deletions(-) >> >> diff --git a/arch/powerpc/platforms/maple/setup.c b/arch/powerpc/platforms/maple/setup.c >> index bfd60e4..ca8a3ff 100644 >> --- a/arch/powerpc/platforms/maple/setup.c >> +++ b/arch/powerpc/platforms/maple/setup.c >> @@ -335,3 +335,50 @@ define_machine(maple) { >> .progress = maple_progress, >> .power_save = power4_idle, >> }; >> + >> +#ifdef CONFIG_EDAC >> +#define CPC925_MC_START 0xf8000000 >> +#define CPC925_MC_END 0xf8ffffff /* sizeof 16MB */ >> +/* Register a platform device for CPC925 memory controller */ >> +static int __init maple_cpc925_edac_setup(void) >> +{ >> + struct platform_device *pdev; >> + struct device_node *np = NULL; >> + struct resource r; >> + >> + np = of_find_node_by_name(NULL, "hostbridge"); >> > > This is way too generic IMHO. You're going to match on any node called > "hostbridge" on any 64-bit powerpc machine, there's a good chance there > are other firmwares out there with such a node, or will be in the > future. > > Is there no compatible or model property you can look for, so you're at > least somewhat confident you're actually looking at a CPC925? > > If not this should be a machine_initcall() for Maple only. > > >> + if (of_address_to_resource(np, 0, &r)) { >> + /* >> + * of_address_to_resource() would get the #address-cells >> + * and #size-cells properties for a node from its parent. >> + * On Maple 64bit target hostbridge parent node has specified >> + * both of these two properties to be 2. However, the actual >> + * "cell" value for host bridge node is 1. Since we can't >> + * fix this firmware-generated DTB, we have to setup a >> + * resource structure manually. >> + */ >> + memset(&r, 0, sizeof(r)); >> + r.start = CPC925_MC_START; >> + r.end = CPC925_MC_END; >> + r.name = "hostbridge"; >> + r.flags = IORESOURCE_MEM; >> > > We do have fixups for bogus firmware including maple, look in > prom_init.c:fixup_device_tree_maple(). Although that's for actual > firmware, not a DTB. > > cheers > > Hi Michael, Thanks a lot for your comments! I can see it would be much better than mine. Next whole week I will be on vacation, I will give some serious thoughts about your ideas and improve my patch in v2 after I come back. Best regards, Harry
diff --git a/arch/powerpc/platforms/maple/setup.c b/arch/powerpc/platforms/maple/setup.c index bfd60e4..ca8a3ff 100644 --- a/arch/powerpc/platforms/maple/setup.c +++ b/arch/powerpc/platforms/maple/setup.c @@ -335,3 +335,50 @@ define_machine(maple) { .progress = maple_progress, .power_save = power4_idle, }; + +#ifdef CONFIG_EDAC +#define CPC925_MC_START 0xf8000000 +#define CPC925_MC_END 0xf8ffffff /* sizeof 16MB */ +/* Register a platform device for CPC925 memory controller */ +static int __init maple_cpc925_edac_setup(void) +{ + struct platform_device *pdev; + struct device_node *np = NULL; + struct resource r; + + np = of_find_node_by_name(NULL, "hostbridge"); + if (!np) { + printk(KERN_ERR "%s: Unable to find hostbridge node\n", + __func__); + return -ENODEV; + } + + if (of_address_to_resource(np, 0, &r)) { + /* + * of_address_to_resource() would get the #address-cells + * and #size-cells properties for a node from its parent. + * On Maple 64bit target hostbridge parent node has specified + * both of these two properties to be 2. However, the actual + * "cell" value for host bridge node is 1. Since we can't + * fix this firmware-generated DTB, we have to setup a + * resource structure manually. + */ + memset(&r, 0, sizeof(r)); + r.start = CPC925_MC_START; + r.end = CPC925_MC_END; + r.name = "hostbridge"; + r.flags = IORESOURCE_MEM; + } + + of_node_put(np); + + pdev = platform_device_register_simple("cpc925_edac", 0, &r, 1); + if (IS_ERR(pdev)) + return PTR_ERR(pdev); + + printk(KERN_INFO "%s: CPC925 platform device created\n", __func__); + + return 0; +} +arch_initcall(maple_cpc925_edac_setup); +#endif
Setup a platform device for the CPC925 Memory Controller during system booting up, against which CPC925 MC EDAC driver would be matched. Signed-off-by: Harry Ciao <qingtao.cao@windriver.com> --- arch/powerpc/platforms/maple/setup.c | 47 ++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-)