[PATCH] ARM: kernel: DT cpu map validity check helper function

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Lorenzo Pieralisi]

Since the introduction of /cpu nodes bindings for ARM and the
corresponding parse function arm_dt_init_cpu_maps(), the cpu_logical_map
and the number of possible CPUs are set according to the DT /cpu
nodes entries. Currently most of the existing ARM SMP platforms detect the
number of cores through HW probing in their .smp_init_cpus functions and set
the possible CPU mask accordingly.
This method should be upgraded so that the CPU counting mechanism will be
based on DT, keeping legacy HW probing mechanism as a fall back solution.

In order to implement this fall back solution mechanism, the ARM DT code
should provide a helper function to platforms to check if the cpu map
has been properly initialized through DT. If the check fails the
platform will resort to legacy HW based cores counting mechanism.

This patch implements a trivial flag and a helper function that platform
can call to check whether DT based cpu map initialization and cores count
were completed successfully.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/include/asm/prom.h | 10 ++++++++++
 arch/arm/kernel/devtree.c   |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
index a219227..487614a 100644
--- a/arch/arm/include/asm/prom.h
+++ b/arch/arm/include/asm/prom.h
@@ -18,6 +18,15 @@
 extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
 extern void arm_dt_memblock_reserve(void);
 extern void __init arm_dt_init_cpu_maps(void);
+/*
+ * Return true if cpu map initialization has been
+ * carried out correctly from DT
+ */
+static inline bool __init arm_dt_cpu_map_valid(void)
+{
+	extern bool valid_dt_cpu_map;
+	return valid_dt_cpu_map;
+}
 
 #else /* CONFIG_OF */
 
@@ -28,6 +37,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
 
 static inline void arm_dt_memblock_reserve(void) { }
 static inline void arm_dt_init_cpu_maps(void) { }
+static inline bool __init arm_dt_cpu_map_valid(void) { return false; }
 
 #endif /* CONFIG_OF */
 #endif /* ASMARM_PROM_H */
diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index 70f1bde..c82af3b 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -62,6 +62,10 @@ void __init arm_dt_memblock_reserve(void)
 		memblock_reserve(base, size);
 	}
 }
+/*
+ * Export DT cpu map validity flag to platforms
+ */
+bool valid_dt_cpu_map __initdata;
 
 /*
  * arm_dt_init_cpu_maps - Function retrieves cpu nodes from the device tree
@@ -158,6 +162,7 @@ void __init arm_dt_init_cpu_maps(void)
 	 * a reg property, the DT CPU list can be considered valid and the
 	 * logical map created in smp_setup_processor_id() can be overridden
 	 */
+	valid_dt_cpu_map = true;
 	for (i = 0; i < cpuidx; i++) {
 		set_cpu_possible(i, true);
 		cpu_logical_map(i) = tmp_map[i];
-- 
1.7.12

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Dave Martin]

On Thu, Jan 10, 2013 at 11:48:27AM +0000, Lorenzo Pieralisi wrote:
> Since the introduction of /cpu nodes bindings for ARM and the
> corresponding parse function arm_dt_init_cpu_maps(), the cpu_logical_map
> and the number of possible CPUs are set according to the DT /cpu
> nodes entries. Currently most of the existing ARM SMP platforms detect the
> number of cores through HW probing in their .smp_init_cpus functions and set
> the possible CPU mask accordingly.
> This method should be upgraded so that the CPU counting mechanism will be
> based on DT, keeping legacy HW probing mechanism as a fall back solution.
> 
> In order to implement this fall back solution mechanism, the ARM DT code
> should provide a helper function to platforms to check if the cpu map
> has been properly initialized through DT. If the check fails the
> platform will resort to legacy HW based cores counting mechanism.
> 
> This patch implements a trivial flag and a helper function that platform
> can call to check whether DT based cpu map initialization and cores count
> were completed successfully.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
>  arch/arm/include/asm/prom.h | 10 ++++++++++
>  arch/arm/kernel/devtree.c   |  5 +++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
> index a219227..487614a 100644
> --- a/arch/arm/include/asm/prom.h
> +++ b/arch/arm/include/asm/prom.h
> @@ -18,6 +18,15 @@
>  extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
>  extern void arm_dt_memblock_reserve(void);
>  extern void __init arm_dt_init_cpu_maps(void);
> +/*
> + * Return true if cpu map initialization has been
> + * carried out correctly from DT
> + */
> +static inline bool __init arm_dt_cpu_map_valid(void)
> +{
> +	extern bool valid_dt_cpu_map;
> +	return valid_dt_cpu_map;
> +}
>  
>  #else /* CONFIG_OF */
>  
> @@ -28,6 +37,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
>  
>  static inline void arm_dt_memblock_reserve(void) { }
>  static inline void arm_dt_init_cpu_maps(void) { }
> +static inline bool __init arm_dt_cpu_map_valid(void) { return false; }
>  
>  #endif /* CONFIG_OF */
>  #endif /* ASMARM_PROM_H */
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index 70f1bde..c82af3b 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -62,6 +62,10 @@ void __init arm_dt_memblock_reserve(void)
>  		memblock_reserve(base, size);
>  	}
>  }
> +/*
> + * Export DT cpu map validity flag to platforms
> + */
> +bool valid_dt_cpu_map __initdata;

Is there any possibility of this being useful after boot?
Hopefully not, but maybe there's something I haven't considered.

Cheers
---Dave

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Lorenzo Pieralisi]

On Thu, Jan 10, 2013 at 12:15:14PM +0000, Dave Martin wrote:
> On Thu, Jan 10, 2013 at 11:48:27AM +0000, Lorenzo Pieralisi wrote:
> > Since the introduction of /cpu nodes bindings for ARM and the
> > corresponding parse function arm_dt_init_cpu_maps(), the cpu_logical_map
> > and the number of possible CPUs are set according to the DT /cpu
> > nodes entries. Currently most of the existing ARM SMP platforms detect the
> > number of cores through HW probing in their .smp_init_cpus functions and set
> > the possible CPU mask accordingly.
> > This method should be upgraded so that the CPU counting mechanism will be
> > based on DT, keeping legacy HW probing mechanism as a fall back solution.
> > 
> > In order to implement this fall back solution mechanism, the ARM DT code
> > should provide a helper function to platforms to check if the cpu map
> > has been properly initialized through DT. If the check fails the
> > platform will resort to legacy HW based cores counting mechanism.
> > 
> > This patch implements a trivial flag and a helper function that platform
> > can call to check whether DT based cpu map initialization and cores count
> > were completed successfully.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > ---
> >  arch/arm/include/asm/prom.h | 10 ++++++++++
> >  arch/arm/kernel/devtree.c   |  5 +++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
> > index a219227..487614a 100644
> > --- a/arch/arm/include/asm/prom.h
> > +++ b/arch/arm/include/asm/prom.h
> > @@ -18,6 +18,15 @@
> >  extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
> >  extern void arm_dt_memblock_reserve(void);
> >  extern void __init arm_dt_init_cpu_maps(void);
> > +/*
> > + * Return true if cpu map initialization has been
> > + * carried out correctly from DT
> > + */
> > +static inline bool __init arm_dt_cpu_map_valid(void)
> > +{
> > +	extern bool valid_dt_cpu_map;
> > +	return valid_dt_cpu_map;
> > +}
> >  
> >  #else /* CONFIG_OF */
> >  
> > @@ -28,6 +37,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
> >  
> >  static inline void arm_dt_memblock_reserve(void) { }
> >  static inline void arm_dt_init_cpu_maps(void) { }
> > +static inline bool __init arm_dt_cpu_map_valid(void) { return false; }
> >  
> >  #endif /* CONFIG_OF */
> >  #endif /* ASMARM_PROM_H */
> > diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> > index 70f1bde..c82af3b 100644
> > --- a/arch/arm/kernel/devtree.c
> > +++ b/arch/arm/kernel/devtree.c
> > @@ -62,6 +62,10 @@ void __init arm_dt_memblock_reserve(void)
> >  		memblock_reserve(base, size);
> >  	}
> >  }
> > +/*
> > + * Export DT cpu map validity flag to platforms
> > + */
> > +bool valid_dt_cpu_map __initdata;
> 
> Is there any possibility of this being useful after boot?
> Hopefully not, but maybe there's something I haven't considered.

I do not think so, at least for the use cases I have envisaged at the moment,
but it is certainly something I should consider.

Thanks,
Lorenzo

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Nicolas Pitre]

On Thu, 10 Jan 2013, Lorenzo Pieralisi wrote:

> On Thu, Jan 10, 2013 at 12:15:14PM +0000, Dave Martin wrote:
> > On Thu, Jan 10, 2013 at 11:48:27AM +0000, Lorenzo Pieralisi wrote:
> > > Since the introduction of /cpu nodes bindings for ARM and the
> > > corresponding parse function arm_dt_init_cpu_maps(), the cpu_logical_map
> > > and the number of possible CPUs are set according to the DT /cpu
> > > nodes entries. Currently most of the existing ARM SMP platforms detect the
> > > number of cores through HW probing in their .smp_init_cpus functions and set
> > > the possible CPU mask accordingly.
> > > This method should be upgraded so that the CPU counting mechanism will be
> > > based on DT, keeping legacy HW probing mechanism as a fall back solution.
> > > 
> > > In order to implement this fall back solution mechanism, the ARM DT code
> > > should provide a helper function to platforms to check if the cpu map
> > > has been properly initialized through DT. If the check fails the
> > > platform will resort to legacy HW based cores counting mechanism.
> > > 
> > > This patch implements a trivial flag and a helper function that platform
> > > can call to check whether DT based cpu map initialization and cores count
> > > were completed successfully.
> > > 
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > ---
> > >  arch/arm/include/asm/prom.h | 10 ++++++++++
> > >  arch/arm/kernel/devtree.c   |  5 +++++
> > >  2 files changed, 15 insertions(+)
> > > 
> > > diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
> > > index a219227..487614a 100644
> > > --- a/arch/arm/include/asm/prom.h
> > > +++ b/arch/arm/include/asm/prom.h
> > > @@ -18,6 +18,15 @@
> > >  extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
> > >  extern void arm_dt_memblock_reserve(void);
> > >  extern void __init arm_dt_init_cpu_maps(void);
> > > +/*
> > > + * Return true if cpu map initialization has been
> > > + * carried out correctly from DT
> > > + */
> > > +static inline bool __init arm_dt_cpu_map_valid(void)
> > > +{
> > > +	extern bool valid_dt_cpu_map;
> > > +	return valid_dt_cpu_map;
> > > +}
> > >  
> > >  #else /* CONFIG_OF */
> > >  
> > > @@ -28,6 +37,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
> > >  
> > >  static inline void arm_dt_memblock_reserve(void) { }
> > >  static inline void arm_dt_init_cpu_maps(void) { }
> > > +static inline bool __init arm_dt_cpu_map_valid(void) { return false; }
> > >  
> > >  #endif /* CONFIG_OF */
> > >  #endif /* ASMARM_PROM_H */
> > > diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> > > index 70f1bde..c82af3b 100644
> > > --- a/arch/arm/kernel/devtree.c
> > > +++ b/arch/arm/kernel/devtree.c
> > > @@ -62,6 +62,10 @@ void __init arm_dt_memblock_reserve(void)
> > >  		memblock_reserve(base, size);
> > >  	}
> > >  }
> > > +/*
> > > + * Export DT cpu map validity flag to platforms
> > > + */
> > > +bool valid_dt_cpu_map __initdata;
> > 
> > Is there any possibility of this being useful after boot?
> > Hopefully not, but maybe there's something I haven't considered.
> 
> I do not think so, at least for the use cases I have envisaged at the moment,
> but it is certainly something I should consider.

Let's simply adjust the code in the future if that ever becomes the 
case.


Nicolas

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Lorenzo Pieralisi]

On Thu, Jan 10, 2013 at 04:16:43PM +0000, Nicolas Pitre wrote:
> On Thu, 10 Jan 2013, Lorenzo Pieralisi wrote:
> 
> > On Thu, Jan 10, 2013 at 12:15:14PM +0000, Dave Martin wrote:
> > > On Thu, Jan 10, 2013 at 11:48:27AM +0000, Lorenzo Pieralisi wrote:
> > > > Since the introduction of /cpu nodes bindings for ARM and the
> > > > corresponding parse function arm_dt_init_cpu_maps(), the cpu_logical_map
> > > > and the number of possible CPUs are set according to the DT /cpu
> > > > nodes entries. Currently most of the existing ARM SMP platforms detect the
> > > > number of cores through HW probing in their .smp_init_cpus functions and set
> > > > the possible CPU mask accordingly.
> > > > This method should be upgraded so that the CPU counting mechanism will be
> > > > based on DT, keeping legacy HW probing mechanism as a fall back solution.
> > > > 
> > > > In order to implement this fall back solution mechanism, the ARM DT code
> > > > should provide a helper function to platforms to check if the cpu map
> > > > has been properly initialized through DT. If the check fails the
> > > > platform will resort to legacy HW based cores counting mechanism.
> > > > 
> > > > This patch implements a trivial flag and a helper function that platform
> > > > can call to check whether DT based cpu map initialization and cores count
> > > > were completed successfully.
> > > > 
> > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > ---
> > > >  arch/arm/include/asm/prom.h | 10 ++++++++++
> > > >  arch/arm/kernel/devtree.c   |  5 +++++
> > > >  2 files changed, 15 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
> > > > index a219227..487614a 100644
> > > > --- a/arch/arm/include/asm/prom.h
> > > > +++ b/arch/arm/include/asm/prom.h
> > > > @@ -18,6 +18,15 @@
> > > >  extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
> > > >  extern void arm_dt_memblock_reserve(void);
> > > >  extern void __init arm_dt_init_cpu_maps(void);
> > > > +/*
> > > > + * Return true if cpu map initialization has been
> > > > + * carried out correctly from DT
> > > > + */
> > > > +static inline bool __init arm_dt_cpu_map_valid(void)
> > > > +{
> > > > +	extern bool valid_dt_cpu_map;
> > > > +	return valid_dt_cpu_map;
> > > > +}
> > > >  
> > > >  #else /* CONFIG_OF */
> > > >  
> > > > @@ -28,6 +37,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
> > > >  
> > > >  static inline void arm_dt_memblock_reserve(void) { }
> > > >  static inline void arm_dt_init_cpu_maps(void) { }
> > > > +static inline bool __init arm_dt_cpu_map_valid(void) { return false; }
> > > >  
> > > >  #endif /* CONFIG_OF */
> > > >  #endif /* ASMARM_PROM_H */
> > > > diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> > > > index 70f1bde..c82af3b 100644
> > > > --- a/arch/arm/kernel/devtree.c
> > > > +++ b/arch/arm/kernel/devtree.c
> > > > @@ -62,6 +62,10 @@ void __init arm_dt_memblock_reserve(void)
> > > >  		memblock_reserve(base, size);
> > > >  	}
> > > >  }
> > > > +/*
> > > > + * Export DT cpu map validity flag to platforms
> > > > + */
> > > > +bool valid_dt_cpu_map __initdata;
> > > 
> > > Is there any possibility of this being useful after boot?
> > > Hopefully not, but maybe there's something I haven't considered.
> > 
> > I do not think so, at least for the use cases I have envisaged at the moment,
> > but it is certainly something I should consider.
> 
> Let's simply adjust the code in the future if that ever becomes the 
> case.

I will check against cpu_possible_mask, that will solve the problem and
avoid this niggle.

Thanks,
Lorenzo

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Rob Herring]

On 01/10/2013 05:48 AM, Lorenzo Pieralisi wrote:
> Since the introduction of /cpu nodes bindings for ARM and the
> corresponding parse function arm_dt_init_cpu_maps(), the cpu_logical_map
> and the number of possible CPUs are set according to the DT /cpu
> nodes entries. Currently most of the existing ARM SMP platforms detect the
> number of cores through HW probing in their .smp_init_cpus functions and set
> the possible CPU mask accordingly.
> This method should be upgraded so that the CPU counting mechanism will be
> based on DT, keeping legacy HW probing mechanism as a fall back solution.
> 
> In order to implement this fall back solution mechanism, the ARM DT code
> should provide a helper function to platforms to check if the cpu map
> has been properly initialized through DT. If the check fails the
> platform will resort to legacy HW based cores counting mechanism.
> 
> This patch implements a trivial flag and a helper function that platform
> can call to check whether DT based cpu map initialization and cores count
> were completed successfully.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
>  arch/arm/include/asm/prom.h | 10 ++++++++++
>  arch/arm/kernel/devtree.c   |  5 +++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
> index a219227..487614a 100644
> --- a/arch/arm/include/asm/prom.h
> +++ b/arch/arm/include/asm/prom.h
> @@ -18,6 +18,15 @@
>  extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
>  extern void arm_dt_memblock_reserve(void);
>  extern void __init arm_dt_init_cpu_maps(void);
> +/*
> + * Return true if cpu map initialization has been
> + * carried out correctly from DT
> + */
> +static inline bool __init arm_dt_cpu_map_valid(void)
> +{
> +	extern bool valid_dt_cpu_map;
> +	return valid_dt_cpu_map;
> +}
>  
>  #else /* CONFIG_OF */
>  
> @@ -28,6 +37,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
>  
>  static inline void arm_dt_memblock_reserve(void) { }
>  static inline void arm_dt_init_cpu_maps(void) { }
> +static inline bool __init arm_dt_cpu_map_valid(void) { return false; }
>  
>  #endif /* CONFIG_OF */
>  #endif /* ASMARM_PROM_H */
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index 70f1bde..c82af3b 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -62,6 +62,10 @@ void __init arm_dt_memblock_reserve(void)
>  		memblock_reserve(base, size);
>  	}
>  }
> +/*
> + * Export DT cpu map validity flag to platforms
> + */
> +bool valid_dt_cpu_map __initdata;
>  
>  /*
>   * arm_dt_init_cpu_maps - Function retrieves cpu nodes from the device tree
> @@ -158,6 +162,7 @@ void __init arm_dt_init_cpu_maps(void)
>  	 * a reg property, the DT CPU list can be considered valid and the
>  	 * logical map created in smp_setup_processor_id() can be overridden
>  	 */
> +	valid_dt_cpu_map = true;
>  	for (i = 0; i < cpuidx; i++) {
>  		set_cpu_possible(i, true);

I don't see why we need another variable. Doesn't cpu_possible mask
being set here imply that the DT map is valid.

We should start requiring valid /cpus nodes and the legacy fall-back
should be temporary.

Rob

>  		cpu_logical_map(i) = tmp_map[i];
> 

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Lorenzo Pieralisi]

On Thu, Jan 10, 2013 at 04:43:03PM +0000, Rob Herring wrote:
> 
> 
> On 01/10/2013 05:48 AM, Lorenzo Pieralisi wrote:
> > Since the introduction of /cpu nodes bindings for ARM and the
> > corresponding parse function arm_dt_init_cpu_maps(), the cpu_logical_map
> > and the number of possible CPUs are set according to the DT /cpu
> > nodes entries. Currently most of the existing ARM SMP platforms detect the
> > number of cores through HW probing in their .smp_init_cpus functions and set
> > the possible CPU mask accordingly.
> > This method should be upgraded so that the CPU counting mechanism will be
> > based on DT, keeping legacy HW probing mechanism as a fall back solution.
> > 
> > In order to implement this fall back solution mechanism, the ARM DT code
> > should provide a helper function to platforms to check if the cpu map
> > has been properly initialized through DT. If the check fails the
> > platform will resort to legacy HW based cores counting mechanism.
> > 
> > This patch implements a trivial flag and a helper function that platform
> > can call to check whether DT based cpu map initialization and cores count
> > were completed successfully.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > ---
> >  arch/arm/include/asm/prom.h | 10 ++++++++++
> >  arch/arm/kernel/devtree.c   |  5 +++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
> > index a219227..487614a 100644
> > --- a/arch/arm/include/asm/prom.h
> > +++ b/arch/arm/include/asm/prom.h
> > @@ -18,6 +18,15 @@
> >  extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
> >  extern void arm_dt_memblock_reserve(void);
> >  extern void __init arm_dt_init_cpu_maps(void);
> > +/*
> > + * Return true if cpu map initialization has been
> > + * carried out correctly from DT
> > + */
> > +static inline bool __init arm_dt_cpu_map_valid(void)
> > +{
> > +	extern bool valid_dt_cpu_map;
> > +	return valid_dt_cpu_map;
> > +}
> >  
> >  #else /* CONFIG_OF */
> >  
> > @@ -28,6 +37,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
> >  
> >  static inline void arm_dt_memblock_reserve(void) { }
> >  static inline void arm_dt_init_cpu_maps(void) { }
> > +static inline bool __init arm_dt_cpu_map_valid(void) { return false; }
> >  
> >  #endif /* CONFIG_OF */
> >  #endif /* ASMARM_PROM_H */
> > diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> > index 70f1bde..c82af3b 100644
> > --- a/arch/arm/kernel/devtree.c
> > +++ b/arch/arm/kernel/devtree.c
> > @@ -62,6 +62,10 @@ void __init arm_dt_memblock_reserve(void)
> >  		memblock_reserve(base, size);
> >  	}
> >  }
> > +/*
> > + * Export DT cpu map validity flag to platforms
> > + */
> > +bool valid_dt_cpu_map __initdata;
> >  
> >  /*
> >   * arm_dt_init_cpu_maps - Function retrieves cpu nodes from the device tree
> > @@ -158,6 +162,7 @@ void __init arm_dt_init_cpu_maps(void)
> >  	 * a reg property, the DT CPU list can be considered valid and the
> >  	 * logical map created in smp_setup_processor_id() can be overridden
> >  	 */
> > +	valid_dt_cpu_map = true;
> >  	for (i = 0; i < cpuidx; i++) {
> >  		set_cpu_possible(i, true);
> 
> I don't see why we need another variable. Doesn't cpu_possible mask
> being set here imply that the DT map is valid.

Yes, that's correct I can use cpu_possible mask to check that, but I still
want to do it through a function/macro, I do not want platform code to
check possible mask explicitly, they have to do it through a helper function.

I will remove the variable.

> We should start requiring valid /cpus nodes and the legacy fall-back
> should be temporary.

I could not agree more.

Thanks,
Lorenzo

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Lorenzo Pieralisi]

Since the introduction of /cpu nodes bindings for ARM and the
corresponding parse function arm_dt_init_cpu_maps(), the cpu_logical_map
and the number of possible CPUs are set according to the DT /cpu
nodes entries. Currently most of the existing ARM SMP platforms detect the
number of cores through HW probing in their .smp_init_cpus functions and set
the possible CPU mask accordingly.
This method should be upgraded so that the CPU counting mechanism will be
based on DT, keeping legacy HW probing mechanism as a fall back solution.

In order to implement this fall back solution mechanism, the ARM DT code
should provide a helper function to platforms to check if the cpu map
has been properly initialized through DT. If the check fails the
platform will resort to legacy HW based cores counting mechanism.

This patch implements a helper function that platforms can call to check
whether DT based cpu map initialization and cores count were completed
successfully.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/include/asm/prom.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/include/asm/prom.h b/arch/arm/include/asm/prom.h
index a219227..a913989 100644
--- a/arch/arm/include/asm/prom.h
+++ b/arch/arm/include/asm/prom.h
@@ -13,11 +13,21 @@
 
 #define HAVE_ARCH_DEVTREE_FIXUPS
 
+#include <linux/cpumask.h>
+
 #ifdef CONFIG_OF
 
 extern struct machine_desc *setup_machine_fdt(unsigned int dt_phys);
 extern void arm_dt_memblock_reserve(void);
 extern void __init arm_dt_init_cpu_maps(void);
+/*
+ * Return true if cpu map initialization has been
+ * carried out correctly from DT
+ */
+static inline bool __init arm_dt_cpu_map_valid(void)
+{
+	return !!(num_possible_cpus());
+}
 
 #else /* CONFIG_OF */
 
@@ -28,6 +38,7 @@ static inline struct machine_desc *setup_machine_fdt(unsigned int dt_phys)
 
 static inline void arm_dt_memblock_reserve(void) { }
 static inline void arm_dt_init_cpu_maps(void) { }
+static inline bool __init arm_dt_cpu_map_valid(void) { return false; }
 
 #endif /* CONFIG_OF */
 #endif /* ASMARM_PROM_H */
-- 
1.7.12

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Russell King - ARM Linux]

On Fri, Jan 11, 2013 at 04:17:38PM +0000, Lorenzo Pieralisi wrote:
> This patch implements a helper function that platforms can call to check
> whether DT based cpu map initialization and cores count were completed
> successfully.

Umm, are you sure this works?  Two problems here:
- the kernel boot marks the booting CPU (in our case, CPU0) as present,
  possible and online before arch code gets called.  smp_init_cpus()
  will be called with the maps already initialized per that.

- this really needs to be paired with a patch showing how you intend it
  to be used; merely adding the helper without any sign of it being used
  means it's just bloat which may not end up being used.

[PATCH] ARM: kernel: DT cpu map validity check helper function

[Lorenzo Pieralisi]

On Fri, Jan 11, 2013 at 04:30:57PM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 11, 2013 at 04:17:38PM +0000, Lorenzo Pieralisi wrote:
> > This patch implements a helper function that platforms can call to check
> > whether DT based cpu map initialization and cores count were completed
> > successfully.
> 
> Umm, are you sure this works?  Two problems here:
> - the kernel boot marks the booting CPU (in our case, CPU0) as present,
>   possible and online before arch code gets called.  smp_init_cpus()
>   will be called with the maps already initialized per that.

Gah, my bad, you are definitely right.

> - this really needs to be paired with a patch showing how you intend it
>   to be used; merely adding the helper without any sign of it being used
>   means it's just bloat which may not end up being used.

Ok, that's a fair point, I will ask to get it merged with an implementation
that relies on it, showing its usage.

Thanks for the review,
Lorenzo