Re: [PATCH v2 2/4] powerpc/mem: Store the dt_root_size/addr cell values for later usage

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

On 8/17/20 9:00 PM, Hari Bathini wrote:
> 
> 
> On 06/08/20 9:53 pm, Aneesh Kumar K.V wrote:
>> dt_root_addr_cells and dt_root_size_cells are __initdata variables.
>> So make a copy of the same which can be used post init.
>>
> 
> This avoids doing the same thing at multiple places.
> So, thanks for the patch, Aneesh.
> 
> Looks good to me.
> 
> but nitpick below...
> 
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>   arch/powerpc/include/asm/drmem.h | 2 ++
>>   arch/powerpc/kernel/prom.c       | 7 +++++++
>>   arch/powerpc/mm/numa.c           | 1 +
>>   3 files changed, 10 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/drmem.h 
>> b/arch/powerpc/include/asm/drmem.h
>> index d719cbac34b2..ffb59caa88ee 100644
>> --- a/arch/powerpc/include/asm/drmem.h
>> +++ b/arch/powerpc/include/asm/drmem.h
>> @@ -123,4 +123,6 @@ static inline void lmb_clear_nid(struct drmem_lmb 
>> *lmb)
>>   }
>>   #endif
>> +extern int mem_addr_cells, mem_size_cells;
> 
> Should this be in include/asm/prom.h instead, given the definition
> comes from kernel/prom.c file?
> 

We added the variable definition to prom.c because that is where we are 
doing early device tree scanning. But the users should not really be 
including prom.h. The variables are related drmem and hence I used 
drmem.h for include.

>> +
>>   #endif /* _ASM_POWERPC_LMB_H */
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index d8a2fb87ba0c..9a1701e85747 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -73,6 +73,7 @@ u64 ppc64_rma_size;
>>   #endif
>>   static phys_addr_t first_memblock_size;
>>   static int __initdata boot_cpu_count;
>> +int mem_addr_cells, mem_size_cells;
>>   static int __init early_parse_mem(char *p)
>>   {
>> @@ -536,6 +537,12 @@ static int __init 
>> early_init_dt_scan_memory_ppc(unsigned long node,
>>                           const char *uname,
>>                           int depth, void *data)
>>   {
>> +    /*
>> +     * Make a copy from __initdata variable
>> +     */
>> +    mem_addr_cells = dt_root_addr_cells;
>> +    mem_size_cells = dt_root_size_cells;
>> +
>>   #ifdef CONFIG_PPC_PSERIES
>>       if (depth == 1 &&
>>           strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0) {
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index 058fee9a0835..77d41d9775d2 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -368,6 +368,7 @@ static void __init get_n_mem_cells(int 
>> *n_addr_cells, int *n_size_cells)
>>       of_node_put(memory);
>>   }
>> +/*  dt_mem_next_cell is __init  */
>>   static unsigned long read_n_cells(int n, const __be32 **buf)
>>   {
>>       unsigned long result = 0;
>>