From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2] xen: arm: remove /xen-core-devices node from dom0 dtb Date: Fri, 22 Nov 2013 16:22:03 +0000 Message-ID: <528F84AB.6070409@linaro.org> References: <1385132821-23807-1-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1385132821-23807-1-git-send-email-ian.campbell@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , xen-devel@lists.xen.org Cc: tim@xen.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 11/22/2013 03:07 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 primary gic nodes in the same location as 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. > - Drop any secondary gics. > > Signed-off-by: Ian Campbell > --- > v2: > Discard secondary gics. > Left parameter as "parent" for now since that is used everywhere. > --- [..] > @@ -792,7 +749,15 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, > DT_MATCH_COMPATIBLE("arm,psci"), > DT_MATCH_PATH("/cpus"), > DT_MATCH_TYPE("memory"), > + { /* sentinel */ }, > + }; > + static const struct dt_device_match gic_matches[] __initconst = > + { > DT_MATCH_GIC, > + { /* sentinel */ }, > + }; > + static const struct dt_device_match timer_matches[] __initconst = > + { > DT_MATCH_TIMER, > { /* sentinel */ }, > }; > @@ -806,11 +771,28 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, > DPRINT("handle %s\n", path); > > /* Skip theses nodes and the sub-nodes */ > - if ( dt_match_node(skip_matches, np ) || > - platform_device_is_blacklisted(np) || > - dt_device_used_by(np) == DOMID_XEN ) > + if ( dt_match_node(skip_matches, np ) ) spurious space before ')' > + { > + DPRINT(" Skip it (matched)\n"); > + return 0; > + } > + if ( platform_device_is_blacklisted(np) ) > { > - DPRINT(" Skip it!\n"); > + DPRINT(" Skip it (blacklisted)\n"); > + return 0; > + } > + > + /* Replace these nodes with our own. Note that the original may be > + * used_by DOMID_XEN so this check comes first. */ > + if ( dt_match_node(gic_matches, np ) ) same here > + return make_gic_node(d, kinfo->fdt, np); > + if ( dt_match_node(timer_matches, np ) ) same here > + return make_timer_node(d, kinfo->fdt, np); > + > + /* Skip nodes used by Xen */ > + if ( dt_device_used_by(np) == DOMID_XEN ) > + { > + DPRINT(" Skip it (used by Xen)\n"); > return 0; > } > [..] Except the minor coding style issue: Acked-by: Julien Grall -- Julien Grall