[v2,6/8] powerpc: introduce early_get_first_memblock_info

On Fri, Jul 26, 2013 at 07:18:01PM -0500, Scott Wood wrote:
> >+ * This function run before early_init_devtree, so we have to init
> >+ * initial_boot_params. Since early_init_dt_scan_memory_ppc will be
> >+ * executed again in early_init_devtree, we have to reinitialize the
> >+ * memblock data before return.
> >+ */
> >+void __init early_get_first_memblock_info(void *params,
> >phys_addr_t *size)
> >+{
> >+	/* Setup flat device-tree pointer */
> >+	initial_boot_params = params;
> >+
> >+	/* Scan memory nodes and rebuild MEMBLOCKs */
> >+	of_scan_flat_dt(early_init_dt_scan_root, NULL);
> >+	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
> >+
> >+	if (size)
> >+		*size = first_memblock_size;
> >+
> >+	/* Undo what early_init_dt_scan_memory_ppc does to memblock */
> >+	memblock_reinit();
> >+}
> >+#endif
> 
> Wouldn't it be simpler to set a flag so that
> early_init_dt_add_memory_arch() doesn't mess with memblocks on the
> first pass?

Do you mean something like this?

Thanks,
Kevin
> 
> -Scott

On Sun, 2013-08-04 at 08:45 +0800, Kevin Hao wrote:
> On Fri, Jul 26, 2013 at 07:18:01PM -0500, Scott Wood wrote:
> > >+ * This function run before early_init_devtree, so we have to init
> > >+ * initial_boot_params. Since early_init_dt_scan_memory_ppc will be
> > >+ * executed again in early_init_devtree, we have to reinitialize the
> > >+ * memblock data before return.
> > >+ */
> > >+void __init early_get_first_memblock_info(void *params,
> > >phys_addr_t *size)
> > >+{
> > >+	/* Setup flat device-tree pointer */
> > >+	initial_boot_params = params;
> > >+
> > >+	/* Scan memory nodes and rebuild MEMBLOCKs */
> > >+	of_scan_flat_dt(early_init_dt_scan_root, NULL);
> > >+	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
> > >+
> > >+	if (size)
> > >+		*size = first_memblock_size;
> > >+
> > >+	/* Undo what early_init_dt_scan_memory_ppc does to memblock */
> > >+	memblock_reinit();
> > >+}
> > >+#endif
> > 
> > Wouldn't it be simpler to set a flag so that
> > early_init_dt_add_memory_arch() doesn't mess with memblocks on the
> > first pass?
> 
> Do you mean something like this?
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 9a69d2d..e861394 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -523,6 +523,10 @@ static int __init early_init_dt_scan_memory_ppc(unsigned long node,
>  
>  void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>  {
> +#if defined(CONFIG_RELOCATABLE) && defined(CONFIG_FSL_BOOKE)
> +	static int first_time = 1;
> +#endif
> +
>  #ifdef CONFIG_PPC64
>  	if (iommu_is_off) {
>  		if (base >= 0x80000000ul)
> @@ -541,6 +545,13 @@ void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>  	}
>  
>  	/* Add the chunk to the MEMBLOCK list */
> +
> +#if defined(CONFIG_RELOCATABLE) && defined(CONFIG_FSL_BOOKE)
> +	if (first_time) {
> +		first_time = 0;
> +		return;
> +	}
> +#endif
>  	memblock_add(base, size);
>  }

I think it'd be clearer for it to be an external variable that gets set
by the relocation code -- plus, the above wouldn't work if this gets
called twice due to having multiple memory regions.

-Scott

On Mon, Aug 05, 2013 at 06:59:28PM -0500, Scott Wood wrote:
> On Sun, 2013-08-04 at 08:45 +0800, Kevin Hao wrote:
> >  	memblock_add(base, size);

<snip>

> >  }
> 
> I think it'd be clearer for it to be an external variable that gets set
> by the relocation code -- plus, the above wouldn't work if this gets
> called twice due to having multiple memory regions.

Got it.

Thanks,
Kevin
> 
> -Scott
> 
> 
>