| Message ID | 20190812222844.9636-4-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5,1/7] xen/arm: pass node to device_tree_for_each_node | expand |
Stefano Stabellini writes: > Improve early_print_info to also print the banks saved in > bootinfo.reserved_mem. Print them right after RESVD, increasing the same > index. > > Since we are at it, also switch the existing RESVD print to use unsigned > int. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com> But, please see NIT below. > --- > Changes in v5: > - switch to unsigned > > Changes in v4: > - new patch > --- > xen/arch/arm/bootfdt.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > index 0b0e22a3d0..32153e6207 100644 > --- a/xen/arch/arm/bootfdt.c > +++ b/xen/arch/arm/bootfdt.c > @@ -337,9 +337,10 @@ static int __init early_scan_node(const void *fdt, > static void __init early_print_info(void) > { > struct meminfo *mi = &bootinfo.mem; > + struct meminfo *mem_resv = &bootinfo.reserved_mem; > struct bootmodules *mods = &bootinfo.modules; > struct bootcmdlines *cmds = &bootinfo.cmdlines; > - int i, nr_rsvd; > + unsigned int i, j, nr_rsvd; > > for ( i = 0; i < mi->nr_banks; i++ ) > printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n", > @@ -361,9 +362,15 @@ static void __init early_print_info(void) > continue; > /* fdt_get_mem_rsv returns length */ > e += s; > - printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n", > + printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", > i, s, e); NIT: I see no reason, why this printk is split into two lines, as nicely fits into one line. > } > + for ( j = 0; j < mem_resv->nr_banks; j++, i++ ) > + { > + printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", i, > + mem_resv->bank[j].start, > + mem_resv->bank[j].start + mem_resv->bank[j].size - 1); > + } > printk("\n"); > for ( i = 0 ; i < cmds->nr_mods; i++ ) > printk("CMDLINE[%"PRIpaddr"]:%s %s\n", cmds->cmdline[i].start,
Hi, On 8/13/19 3:28 PM, Volodymyr Babchuk wrote: > > Stefano Stabellini writes: > >> Improve early_print_info to also print the banks saved in >> bootinfo.reserved_mem. Print them right after RESVD, increasing the same >> index. >> >> Since we are at it, also switch the existing RESVD print to use unsigned >> int. >> >> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com> > > But, please see NIT below. > >> --- >> Changes in v5: >> - switch to unsigned >> >> Changes in v4: >> - new patch >> --- >> xen/arch/arm/bootfdt.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c >> index 0b0e22a3d0..32153e6207 100644 >> --- a/xen/arch/arm/bootfdt.c >> +++ b/xen/arch/arm/bootfdt.c >> @@ -337,9 +337,10 @@ static int __init early_scan_node(const void *fdt, >> static void __init early_print_info(void) >> { >> struct meminfo *mi = &bootinfo.mem; >> + struct meminfo *mem_resv = &bootinfo.reserved_mem; >> struct bootmodules *mods = &bootinfo.modules; >> struct bootcmdlines *cmds = &bootinfo.cmdlines; >> - int i, nr_rsvd; >> + unsigned int i, j, nr_rsvd; >> >> for ( i = 0; i < mi->nr_banks; i++ ) >> printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n", >> @@ -361,9 +362,15 @@ static void __init early_print_info(void) >> continue; >> /* fdt_get_mem_rsv returns length */ >> e += s; >> - printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n", >> + printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", >> i, s, e); > NIT: I see no reason, why this printk is split into two lines, as nicely fits > into one line. Not mentioning the wrong indentation in pretty much all this function ;). I would prefer if we take care of the indentation issues in a patch before this one. Cheers,
Hi Stefano, On 12/08/2019 23:28, Stefano Stabellini wrote: > Improve early_print_info to also print the banks saved in > bootinfo.reserved_mem. Print them right after RESVD, increasing the same > index. > > Since we are at it, also switch the existing RESVD print to use unsigned > int. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > Changes in v5: > - switch to unsigned > > Changes in v4: > - new patch > --- > xen/arch/arm/bootfdt.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > index 0b0e22a3d0..32153e6207 100644 > --- a/xen/arch/arm/bootfdt.c > +++ b/xen/arch/arm/bootfdt.c > @@ -337,9 +337,10 @@ static int __init early_scan_node(const void *fdt, > static void __init early_print_info(void) > { > struct meminfo *mi = &bootinfo.mem; > + struct meminfo *mem_resv = &bootinfo.reserved_mem; > struct bootmodules *mods = &bootinfo.modules; > struct bootcmdlines *cmds = &bootinfo.cmdlines; > - int i, nr_rsvd; > + unsigned int i, j, nr_rsvd; > > for ( i = 0; i < mi->nr_banks; i++ ) > printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n", > @@ -361,9 +362,15 @@ static void __init early_print_info(void) > continue; > /* fdt_get_mem_rsv returns length */ > e += s; > - printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n", > + printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", > i, s, e); Can you add a patch before to fix the indentation within this function? > } > + for ( j = 0; j < mem_resv->nr_banks; j++, i++ ) > + { > + printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", i, > + mem_resv->bank[j].start, > + mem_resv->bank[j].start + mem_resv->bank[j].size - 1); Even if the most of the function is not correctly indented, new code should at least be. Assuming the two are taken into account: Acked-by: Julien Grall <julien.grall@arm.com> > + } > printk("\n"); > for ( i = 0 ; i < cmds->nr_mods; i++ ) > printk("CMDLINE[%"PRIpaddr"]:%s %s\n", cmds->cmdline[i].start, > Cheers,
On Tue, 13 Aug 2019, Julien Grall wrote: > Hi, > > On 8/13/19 3:28 PM, Volodymyr Babchuk wrote: > > > > Stefano Stabellini writes: > > > > > Improve early_print_info to also print the banks saved in > > > bootinfo.reserved_mem. Print them right after RESVD, increasing the same > > > index. > > > > > > Since we are at it, also switch the existing RESVD print to use unsigned > > > int. > > > > > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > > Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com> Thanks > > But, please see NIT below. > > > > > --- > > > Changes in v5: > > > - switch to unsigned > > > > > > Changes in v4: > > > - new patch > > > --- > > > xen/arch/arm/bootfdt.c | 11 +++++++++-- > > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > > > index 0b0e22a3d0..32153e6207 100644 > > > --- a/xen/arch/arm/bootfdt.c > > > +++ b/xen/arch/arm/bootfdt.c > > > @@ -337,9 +337,10 @@ static int __init early_scan_node(const void *fdt, > > > static void __init early_print_info(void) > > > { > > > struct meminfo *mi = &bootinfo.mem; > > > + struct meminfo *mem_resv = &bootinfo.reserved_mem; > > > struct bootmodules *mods = &bootinfo.modules; > > > struct bootcmdlines *cmds = &bootinfo.cmdlines; > > > - int i, nr_rsvd; > > > + unsigned int i, j, nr_rsvd; > > > for ( i = 0; i < mi->nr_banks; i++ ) > > > printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n", > > > @@ -361,9 +362,15 @@ static void __init early_print_info(void) > > > continue; > > > /* fdt_get_mem_rsv returns length */ > > > e += s; > > > - printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n", > > > + printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", > > > i, s, e); > > NIT: I see no reason, why this printk is split into two lines, as nicely > > fits > > into one line. > > Not mentioning the wrong indentation in pretty much all this function ;). I > would prefer if we take care of the indentation issues in a patch before this > one. I'll add a patch
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index 0b0e22a3d0..32153e6207 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -337,9 +337,10 @@ static int __init early_scan_node(const void *fdt, static void __init early_print_info(void) { struct meminfo *mi = &bootinfo.mem; + struct meminfo *mem_resv = &bootinfo.reserved_mem; struct bootmodules *mods = &bootinfo.modules; struct bootcmdlines *cmds = &bootinfo.cmdlines; - int i, nr_rsvd; + unsigned int i, j, nr_rsvd; for ( i = 0; i < mi->nr_banks; i++ ) printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n", @@ -361,9 +362,15 @@ static void __init early_print_info(void) continue; /* fdt_get_mem_rsv returns length */ e += s; - printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n", + printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", i, s, e); } + for ( j = 0; j < mem_resv->nr_banks; j++, i++ ) + { + printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", i, + mem_resv->bank[j].start, + mem_resv->bank[j].start + mem_resv->bank[j].size - 1); + } printk("\n"); for ( i = 0 ; i < cmds->nr_mods; i++ ) printk("CMDLINE[%"PRIpaddr"]:%s %s\n", cmds->cmdline[i].start,
Improve early_print_info to also print the banks saved in bootinfo.reserved_mem. Print them right after RESVD, increasing the same index. Since we are at it, also switch the existing RESVD print to use unsigned int. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> --- Changes in v5: - switch to unsigned Changes in v4: - new patch --- xen/arch/arm/bootfdt.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)