Re: [Qemu-devel] [PATCH v2] arm: add device tree support

[Alexander Graf]

On 01.02.2012, at 18:38, Grant Likely wrote:

> On Tue, Jan 31, 2012 at 6:44 PM, Alexander Graf <address@hidden> wrote:
>> 
>> On 01.02.2012, at 02:35, Paul Brook wrote:
>> 
>>>> We could also just change machine->init() and pass the dtb in there. In a
>>>> QOM world these would become machine device properties anyways.
>>>> 
>>>>    machine->init(ram_size, boot_devices,
>>>>                  kernel_filename, kernel_cmdline, initrd_filename,
>>>> cpu_model);
>>>> 
>>>> Essentially we shouldn't treat -dtb any different than -kernel or -initrd.
>>>> It's also useful for more than ARM, namely embedded ppc systems. But I can
>>>> easily post a follow-up patch for those.
>>> 
>>> Changing machine->init means you have to touch every single board file, and
>>> clone the exact same code for every machine that uses arm_boot.c.  All of
>>> which will be rewritten in the near future.
>> 
>> Well, the dt file name would have to be passed into the generic arm_boot.c 
>> function, yes. But that's something that we need to do at one point in time 
>> either way, because machines will want to have default dtb file names.
>> 
>>> machine->init is a particularly suckiy interface to start with, we want to 
>>> be
>>> using it less, not more.  It's not like we're going support multiple machine
>>> instanced.  At least not before machine->init is removed altogether.
>> 
>> I do see your point on not extending legacy interfaces though and not 
>> bloating up the patch. In fact, I'm indifferent enough on the actual 
>> implementation atm, as long as the command line interface (or whatever the 
>> user sees) is reasonably sane. And it is IMHO. So if it makes everything 
>> easier, do it using a global, but keep in mind that this will need 
>> refactoring.
> 
> That's certainly my expectation.  My initial instinct was also to
> handle it the say way as initrd and kernel pointers, but as Paul
> pointed out it requires touching all init functions which is a dead
> end effort when ->init() gets killed off.  This patch is trivial to
> get the functionality into qemu without making it any more difficult
> for whoever creates the arm-kernel-loader device that Anthony is
> talking about.

Yeah, I agree. Let's separate the QOM efforts from making things work for now. 
I don't want to have yet another if=ahci or hotplug magic where I'm waiting for 
a year for salvation that never came. Let's get the feature in and model the 
whole thing properly with all cases taken into account. This way we at least 
don't forget about dtbs when modeling the arm-kernel-loader device :)


Alex