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....@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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Reply via email to