From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vijay Kilari Subject: Re: [PATCH v2 15/15] xen/arm: update GIC dt node with GIC v3 information Date: Fri, 4 Apr 2014 21:43:28 +0530 Message-ID: References: <1396612593-443-1-git-send-email-vijay.kilari@gmail.com> <1396612593-443-16-git-send-email-vijay.kilari@gmail.com> <533EC014.4030500@linaro.org> <533ED724.1070001@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <533ED724.1070001@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: Ian Campbell , Stefano Stabellini , Prasun Kapoor , Vijaya Kumar K , xen-devel@lists.xen.org, Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Fri, Apr 4, 2014 at 9:30 PM, Julien Grall wrote: > Hello Vijaya, > > On 04/04/2014 04:45 PM, Vijay Kilari wrote: >> On Fri, Apr 4, 2014 at 7:52 PM, Julien Grall wrote: >>> On 04/04/2014 12:56 PM, vijay.kilari@gmail.com wrote: >>>> + if ( hw_type == GIC_VERSION_V3 ) >>>> + { >>>> + res = dt_property_read_u32(gic, "redistributor-stride", &rd_stride); >>>> + if ( !res ) >>>> + rd_stride = 0; >>>> + } >>> >>> >>> You have skipped some of my remarks on V1. I would definitely prefer to >>> have a callback in your {v,}GIC structure which add the specific GIC >>> properties following the version supported by DOM0. >>> >> Yes, I recognized your comment. My understanding is that while >> creating this gic node for >> dom0, we should follow the order in creating cells. >> Also, the cells are allocated here and these should be passed to gic >> driver to fill it. >> So looks ugly. > > I don't understand the issue with the order and the allocation... > I mean if we change the order of calling fdt_set_property for various properties, the node creation goes wrong. something like this fdt_set_property(fdt, "mycommonprop2",..) fdt_set_property(fdt, "mycommonprop1",..) I have given a try and dom0 fails to boot. > You have bunch of code in if (hw_type == GICV3) than can be easily move > to the GIC driver. > > You could have something like: > > fdt_set_property(fdt, "mycommonprop1",..) > fdt_set_property(fdt, "mycommonprop2",..) > gic_node_specific_properties(); > fdt_end(); > >> I prefer completely move this code to gic.c and from gic.c based on >> registered callback, let gic-{v2/v3}.c manage version specific dt cells update. >> Is this OK? > > What do you mean? > I mean why can't we move, why can't we move complete make_gic_node to gic.c? as below and let it handle hw specific things? fdt_set_property(fdt, "mycommonprop1",..) fdt_set_property(fdt, "mycommonprop2",..) gic_node_specific_properties(); fdt_end(); Anyways, I will relook and see if I can simplify as you suggested > Regards, > > -- > Julien Grall