Matthew Daley
2013-Nov-08 00:32 UTC
[PATCH] xen/arm: correct duplicate MPIDR check to actually skip the node
Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
I assume this was what was really intended...
xen/arch/arm/smpboot.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index b836be4..6c90fa6 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -161,9 +161,11 @@ void __init smp_init_cpus(void)
printk(XENLOG_WARNING
"cpu node `%s`: duplicate /cpu reg properties in
the DT\n",
dt_node_full_name(cpu));
- continue;
+ break;
}
}
+ if ( j != cpuidx )
+ continue;
/*
* Build a stashed array of MPIDR values. Numbering scheme requires
--
1.7.10.4
Ian Campbell
2013-Nov-11 16:10 UTC
Re: [PATCH] xen/arm: correct duplicate MPIDR check to actually skip the node
On Fri, 2013-11-08 at 13:32 +1300, Matthew Daley wrote:> Signed-off-by: Matthew Daley <mattjd@gmail.com> > --- > I assume this was what was really intended...CCing Julien... But yes it looks like it.> xen/arch/arm/smpboot.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index b836be4..6c90fa6 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -161,9 +161,11 @@ void __init smp_init_cpus(void) > printk(XENLOG_WARNING > "cpu node `%s`: duplicate /cpu reg properties in the DT\n", > dt_node_full_name(cpu)); > - continue; > + break; > } > } > + if ( j != cpuidx ) > + continue;Took me a moment to figure out what this was for. I''m half minded to suggest this is one of those places where a "goto next_cpu" would have been ok, I''d definitely want a second opinion on that though!> > /* > * Build a stashed array of MPIDR values. Numbering scheme requires
Julien Grall
2013-Nov-11 19:54 UTC
Re: [PATCH] xen/arm: correct duplicate MPIDR check to actually skip the node
On 11/11/2013 04:10 PM, Ian Campbell wrote:> On Fri, 2013-11-08 at 13:32 +1300, Matthew Daley wrote: >> Signed-off-by: Matthew Daley <mattjd@gmail.com> >> --- >> I assume this was what was really intended... >Indeed, that for spotting this error !> CCing Julien... But yes it looks like it. > >> xen/arch/arm/smpboot.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c >> index b836be4..6c90fa6 100644 >> --- a/xen/arch/arm/smpboot.c >> +++ b/xen/arch/arm/smpboot.c >> @@ -161,9 +161,11 @@ void __init smp_init_cpus(void) >> printk(XENLOG_WARNING >> "cpu node `%s`: duplicate /cpu reg properties in the DT\n", >> dt_node_full_name(cpu)); >> - continue; >> + break; >> } >> } >> + if ( j != cpuidx ) >> + continue; > > Took me a moment to figure out what this was for. I''m half minded to > suggest this is one of those places where a "goto next_cpu" would have > been ok, I''d definitely want a second opinion on that though! >I''m happy with the both solutions: Acked-by: Julien Grall <julien.grall@linaro.org> -- Julien Grall
Ian Campbell
2013-Nov-19 14:53 UTC
Re: [PATCH] xen/arm: correct duplicate MPIDR check to actually skip the node
On Mon, 2013-11-11 at 19:54 +0000, Julien Grall wrote:> On 11/11/2013 04:10 PM, Ian Campbell wrote: > > On Fri, 2013-11-08 at 13:32 +1300, Matthew Daley wrote: > >> Signed-off-by: Matthew Daley <mattjd@gmail.com> > >> --- > >> I assume this was what was really intended... > > > > Indeed, that for spotting this error ! > > > CCing Julien... But yes it looks like it. > > > >> xen/arch/arm/smpboot.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > >> index b836be4..6c90fa6 100644 > >> --- a/xen/arch/arm/smpboot.c > >> +++ b/xen/arch/arm/smpboot.c > >> @@ -161,9 +161,11 @@ void __init smp_init_cpus(void) > >> printk(XENLOG_WARNING > >> "cpu node `%s`: duplicate /cpu reg properties in the DT\n", > >> dt_node_full_name(cpu)); > >> - continue; > >> + break; > >> } > >> } > >> + if ( j != cpuidx ) > >> + continue; > > > > Took me a moment to figure out what this was for. I''m half minded to > > suggest this is one of those places where a "goto next_cpu" would have > > been ok, I''d definitely want a second opinion on that though! > > > > I''m happy with the both solutions: > > Acked-by: Julien Grall <julien.grall@linaro.org>Applied. thanks.