Re: [Qemu-devel] [PATCH 15/25] PPC: e500: dt: create /soc8544 node dynamically

[Alexander Graf]

On 31.05.2012, at 23:58, Scott Wood wrote:

> On 05/30/2012 06:00 AM, Alexander Graf wrote:
>> @@ -179,6 +182,20 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
>>         }
>>     }
>> 
>> +    /* XXX These should go into their respective devices' code */
>> +    sprintf(soc, "/address@hidden", MPC8544_CCSRBAR_BASE);
> 
> This should just be "/address@hidden" (something like "address@hidden" or 
> "address@hidden"
> would be better, but hasn't really caught on yet).  "soc8544"-type names
> have been discouraged for a while now.
> 
>> +    qemu_devtree_setprop_string(fdt, soc, "compatible", "simple-bus");
> 
> It should also have a compatible of "fsl,mpc8544-immr".
> 
>> +    qemu_devtree_setprop_cell2(fdt, soc, "reg", MPC8544_CCSRBAR_BASE,
>> +                               MPC8544_CCSRBAR_REGSIZE);
> 
> We don't need reg here anymore.
> 
> I realize this was probably meant to produce the same output as the
> current device tree, but if we're introducing new code to generate this
> stuff we should follow current best practices.

Good point. I guess we should clean that up as a patch at the end of the series 
though. First convert the state 1:1 over, then improve it.


Alex