Re: [PATCH] xen: arm: remove /xen-core-devices node from dom0 dtb

[Julien Grall <julien.grall@linaro.org>]

On 11/22/2013 02:40 PM, Ian Campbell wrote:
> On Fri, 2013-11-22 at 14:18 +0000, Julien Grall wrote:
>>
>> On 11/22/2013 01:47 PM, Ian Campbell wrote:
>>> The intention of adding this node to contain the GIC, timer and memory nodes
>>> (in 1c08d6004ea7) was to allow us to control the #address-cells and
>>>
>>> However in the case of the memory node the #*-cells are always taken from the
>>> root node (see ePAPR 3.1, "the following nodes shall be present at the root...
>>> memory node"). This caused breakage on the arndale platform.
>>>
>>> In addition it is not valid to just create sub-nodes like this. Unless they
>>> declare themselves as a bus then they will not necessarily be enumerated
>>> (although Linux currently does so in practice).
>>>
>>> Therefore:
>>>    - Move the memory node back to the top level.
>>>    - Insert the timer and gic nodes in the same location as in the host DTB,
>>>      replacing the originals. Note that the nodes here may be marked as in use
>>>      by Xen and therefore the check must be before we discard nodes used by Xen.
>>
>> If you insert the GIC and timer nodes at the same location you need to
>> make sure the use the right value. For instance it may happen to have
>> multiple GIC in the device tree.
>> Actually, Xen skips all the GIC nodes and create just one. Here, you
>> replace all the possible GIC nodes by the same value (I'm not sure how
>> Linux will deal with that).
>
> Damn, I forgot about this case. I'm not sure how relevant it is to us
> today though -- do we support any such systems and/or would they be
> likely to just work, I would expect not?

We don't support this kind of systems. Xen only handle one GIC at time 
in arch/arm/gic.c.
During dom0 DT building we will just remove all GIC nodes and just 
create one (the main controller).

>
>> For now, I think the best the solution is to check if we are the main
>> interrupt controller. If not, the function just need to return 0.
>
> Means checking dt_interrupt_controller->phandle against the node in
> hand, right?

You give in argument the current node pointer (ie the GIC). So you can 
compare this pointer to dt_interrupt_controller.

BTW, the name of the argument "parent" is wrong. It's not the parent but 
the current node. I would rename to "np".

-- 
Julien Grall