[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 2/2] xen/arm: merge make_timer_node and make_timer_domU_node
Hi, NIT: s/merge/consolidate/ On 31/07/2019 11:28, Viktor Mitin wrote: Merged make_timer_node and make_timer_domU_node into one function make_timer_node. Kept the domU version for the compatible as it is simpler. Kept the hw version for the clock as it is relevant for the both cases. The commit message needs a bit of rewording: - It is not clear why they the two functions are merged - This needs more word around so the commit message looks like a coherent text. Suggested-by: Julien Grall <julien.grall@xxxxxxx> Signed-off-by: Viktor Mitin <viktor_mitin@xxxxxxxx> --- v4 updates: updated "Kept the domU version for the compatible as it is simpler" xen/arch/arm/domain_build.c | 109 +++++++++++++----------------------- 1 file changed, 39 insertions(+), 70 deletions(-) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index d04a1c3aec..4d7c3411a6 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -964,8 +964,12 @@ static int __init make_gic_node(const struct domain *d, void *fdt,static int __init make_timer_node(const struct kernel_info *kinfo){ + int res; void *fdt = kinfo->fdt; - Please avoid to add code that you drop in a patch later. + unsigned int irq[MAX_TIMER_PPI]; + gic_interrupt_t intrs[3]; + u32 clock_frequency; + bool clock_valid; This is not related to this patch and only increase the complexity of the review. If you want to do reshuffling then it should be a separate patch. But then, I see you real value of the re-ordering here. static const struct dt_device_match timer_ids[] __initconst = { DT_MATCH_COMPATIBLE("arm,armv7-timer"), @@ -973,15 +977,6 @@ static int __init make_timer_node(const struct kernel_info *kinfo) { /* sentinel */ }, }; struct dt_device_node *dev; - u32 len; - const void *compatible; - int res; - unsigned int irq; - gic_interrupt_t intrs[3]; - u32 clock_frequency; - bool clock_valid; - - dt_dprintk("Create timer node\n"); Why is it dropped? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |