Message ID | 20190130014216.98287-1-aik@ozlabs.ru |
---|---|
State | New |
Headers | show |
Series | [qemu] spapr: Drop unused parameters from fdt building helper | expand |
On Wed, 30 Jan 2019 12:42:16 +1100 Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > spapr_load_rtas() handles now RTAS address and size information in the FDT > so drop them from spapr_build_fdt(). > > While we are here, fix a small typo. > > Fixes: 2cac78c12ade9 "pseries: Consolidate RTAS loading" One nit. The last rtas_* user in spapr_build_fdt() was removed by the following hunk: @@ -949,12 +966,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr, } } - /* RTAS */ - ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size); - if (ret < 0) { - error_report("Couldn't set up RTAS device tree properties"); - } - /* cpus */ spapr_populate_cpus_dt_node(fdt, spapr); from commit: 3f5dabceba24 "pseries: Consolidate construction of /rtas device tree node" Reviewed-by: Greg Kurz <groug@kaod.org> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > hw/ppc/spapr.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index a217c7f..fa12723 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1225,9 +1225,7 @@ static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt) > } > } > > -static void *spapr_build_fdt(sPAPRMachineState *spapr, > - hwaddr rtas_addr, > - hwaddr rtas_size) > +static void *spapr_build_fdt(sPAPRMachineState *spapr) > { > MachineState *machine = MACHINE(spapr); > MachineClass *mc = MACHINE_GET_CLASS(machine); > @@ -1644,14 +1642,14 @@ static void spapr_machine_reset(void) > > /* > * We place the device tree and RTAS just below either the top of the RMA, > - * or just below 2GB, whichever is lowere, so that it can be > + * or just below 2GB, whichever is lower, so that it can be > * processed with 32-bit real mode code if necessary > */ > rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR); > rtas_addr = rtas_limit - RTAS_MAX_SIZE; > fdt_addr = rtas_addr - FDT_MAX_SIZE; > > - fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size); > + fdt = spapr_build_fdt(spapr); > > spapr_load_rtas(spapr, fdt, rtas_addr); >
Hi Alexey, On 1/30/19 1:43 PM, Greg Kurz wrote: > On Wed, 30 Jan 2019 12:42:16 +1100 > Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > >> spapr_load_rtas() handles now RTAS address and size information in the FDT >> so drop them from spapr_build_fdt(). >> >> While we are here, fix a small typo. >> >> Fixes: 2cac78c12ade9 "pseries: Consolidate RTAS loading" > > One nit. The last rtas_* user in spapr_build_fdt() was removed by the > following hunk: > > @@ -949,12 +966,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr, > } > } > > - /* RTAS */ > - ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size); > - if (ret < 0) { > - error_report("Couldn't set up RTAS device tree properties"); > - } > - > /* cpus */ > spapr_populate_cpus_dt_node(fdt, spapr); > > from commit: > > 3f5dabceba24 "pseries: Consolidate construction of /rtas device tree node" Can you (or David if he agrees) add a line about since when/why it is no more required? > Reviewed-by: Greg Kurz <groug@kaod.org> > >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> hw/ppc/spapr.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index a217c7f..fa12723 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -1225,9 +1225,7 @@ static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt) >> } >> } >> >> -static void *spapr_build_fdt(sPAPRMachineState *spapr, >> - hwaddr rtas_addr, >> - hwaddr rtas_size) >> +static void *spapr_build_fdt(sPAPRMachineState *spapr) >> { >> MachineState *machine = MACHINE(spapr); >> MachineClass *mc = MACHINE_GET_CLASS(machine); >> @@ -1644,14 +1642,14 @@ static void spapr_machine_reset(void) >> >> /* >> * We place the device tree and RTAS just below either the top of the RMA, >> - * or just below 2GB, whichever is lowere, so that it can be >> + * or just below 2GB, whichever is lower, so that it can be >> * processed with 32-bit real mode code if necessary >> */ >> rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR); >> rtas_addr = rtas_limit - RTAS_MAX_SIZE; >> fdt_addr = rtas_addr - FDT_MAX_SIZE; >> >> - fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size); >> + fdt = spapr_build_fdt(spapr); >> >> spapr_load_rtas(spapr, fdt, rtas_addr); >> > >
On Wed, 30 Jan 2019 17:43:44 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Alexey, > > On 1/30/19 1:43 PM, Greg Kurz wrote: > > On Wed, 30 Jan 2019 12:42:16 +1100 > > Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > > > >> spapr_load_rtas() handles now RTAS address and size information in the FDT > >> so drop them from spapr_build_fdt(). > >> > >> While we are here, fix a small typo. > >> > >> Fixes: 2cac78c12ade9 "pseries: Consolidate RTAS loading" > > > > One nit. The last rtas_* user in spapr_build_fdt() was removed by the > > following hunk: > > > > @@ -949,12 +966,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr, > > } > > } > > > > - /* RTAS */ > > - ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size); > > - if (ret < 0) { > > - error_report("Couldn't set up RTAS device tree properties"); > > - } > > - > > /* cpus */ > > spapr_populate_cpus_dt_node(fdt, spapr); > > > > from commit: > > > > 3f5dabceba24 "pseries: Consolidate construction of /rtas device tree node" > > Can you (or David if he agrees) add a line about since when/why it is no > more required? > Commit 3f5dabceba24 (first released in QEMU 2.10) simply moved the FDT code for RTAS to some other function, leaving the rtas_addr and rtas_size arguments unused in spapr_build_fdt(). This patch is a trivial cleanup really. > > Reviewed-by: Greg Kurz <groug@kaod.org> > > > >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > >> --- > >> hw/ppc/spapr.c | 8 +++----- > >> 1 file changed, 3 insertions(+), 5 deletions(-) > >> > >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > >> index a217c7f..fa12723 100644 > >> --- a/hw/ppc/spapr.c > >> +++ b/hw/ppc/spapr.c > >> @@ -1225,9 +1225,7 @@ static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt) > >> } > >> } > >> > >> -static void *spapr_build_fdt(sPAPRMachineState *spapr, > >> - hwaddr rtas_addr, > >> - hwaddr rtas_size) > >> +static void *spapr_build_fdt(sPAPRMachineState *spapr) > >> { > >> MachineState *machine = MACHINE(spapr); > >> MachineClass *mc = MACHINE_GET_CLASS(machine); > >> @@ -1644,14 +1642,14 @@ static void spapr_machine_reset(void) > >> > >> /* > >> * We place the device tree and RTAS just below either the top of the RMA, > >> - * or just below 2GB, whichever is lowere, so that it can be > >> + * or just below 2GB, whichever is lower, so that it can be > >> * processed with 32-bit real mode code if necessary > >> */ > >> rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR); > >> rtas_addr = rtas_limit - RTAS_MAX_SIZE; > >> fdt_addr = rtas_addr - FDT_MAX_SIZE; > >> > >> - fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size); > >> + fdt = spapr_build_fdt(spapr); > >> > >> spapr_load_rtas(spapr, fdt, rtas_addr); > >> > > > >
On Wed, Jan 30, 2019 at 05:53:16PM +0100, Greg Kurz wrote: > On Wed, 30 Jan 2019 17:43:44 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > Hi Alexey, > > > > On 1/30/19 1:43 PM, Greg Kurz wrote: > > > On Wed, 30 Jan 2019 12:42:16 +1100 > > > Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > > > > > >> spapr_load_rtas() handles now RTAS address and size information in the FDT > > >> so drop them from spapr_build_fdt(). > > >> > > >> While we are here, fix a small typo. > > >> > > >> Fixes: 2cac78c12ade9 "pseries: Consolidate RTAS loading" > > > > > > One nit. The last rtas_* user in spapr_build_fdt() was removed by the > > > following hunk: > > > > > > @@ -949,12 +966,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr, > > > } > > > } > > > > > > - /* RTAS */ > > > - ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size); > > > - if (ret < 0) { > > > - error_report("Couldn't set up RTAS device tree properties"); > > > - } > > > - > > > /* cpus */ > > > spapr_populate_cpus_dt_node(fdt, spapr); > > > > > > from commit: > > > > > > 3f5dabceba24 "pseries: Consolidate construction of /rtas device tree node" > > > > Can you (or David if he agrees) add a line about since when/why it is no > > more required? > > > > Commit 3f5dabceba24 (first released in QEMU 2.10) simply moved the FDT code > for RTAS to some other function, leaving the rtas_addr and rtas_size arguments > unused in spapr_build_fdt(). This patch is a trivial cleanup really. Applied to ppc-for-4.0, with the Fixes line adjusted as suggested. > > > > Reviewed-by: Greg Kurz <groug@kaod.org> > > > > > >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > > >> --- > > >> hw/ppc/spapr.c | 8 +++----- > > >> 1 file changed, 3 insertions(+), 5 deletions(-) > > >> > > >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > >> index a217c7f..fa12723 100644 > > >> --- a/hw/ppc/spapr.c > > >> +++ b/hw/ppc/spapr.c > > >> @@ -1225,9 +1225,7 @@ static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt) > > >> } > > >> } > > >> > > >> -static void *spapr_build_fdt(sPAPRMachineState *spapr, > > >> - hwaddr rtas_addr, > > >> - hwaddr rtas_size) > > >> +static void *spapr_build_fdt(sPAPRMachineState *spapr) > > >> { > > >> MachineState *machine = MACHINE(spapr); > > >> MachineClass *mc = MACHINE_GET_CLASS(machine); > > >> @@ -1644,14 +1642,14 @@ static void spapr_machine_reset(void) > > >> > > >> /* > > >> * We place the device tree and RTAS just below either the top of the RMA, > > >> - * or just below 2GB, whichever is lowere, so that it can be > > >> + * or just below 2GB, whichever is lower, so that it can be > > >> * processed with 32-bit real mode code if necessary > > >> */ > > >> rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR); > > >> rtas_addr = rtas_limit - RTAS_MAX_SIZE; > > >> fdt_addr = rtas_addr - FDT_MAX_SIZE; > > >> > > >> - fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size); > > >> + fdt = spapr_build_fdt(spapr); > > >> > > >> spapr_load_rtas(spapr, fdt, rtas_addr); > > >> > > > > > > >
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a217c7f..fa12723 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1225,9 +1225,7 @@ static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt) } } -static void *spapr_build_fdt(sPAPRMachineState *spapr, - hwaddr rtas_addr, - hwaddr rtas_size) +static void *spapr_build_fdt(sPAPRMachineState *spapr) { MachineState *machine = MACHINE(spapr); MachineClass *mc = MACHINE_GET_CLASS(machine); @@ -1644,14 +1642,14 @@ static void spapr_machine_reset(void) /* * We place the device tree and RTAS just below either the top of the RMA, - * or just below 2GB, whichever is lowere, so that it can be + * or just below 2GB, whichever is lower, so that it can be * processed with 32-bit real mode code if necessary */ rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR); rtas_addr = rtas_limit - RTAS_MAX_SIZE; fdt_addr = rtas_addr - FDT_MAX_SIZE; - fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size); + fdt = spapr_build_fdt(spapr); spapr_load_rtas(spapr, fdt, rtas_addr);
spapr_load_rtas() handles now RTAS address and size information in the FDT so drop them from spapr_build_fdt(). While we are here, fix a small typo. Fixes: 2cac78c12ade9 "pseries: Consolidate RTAS loading" Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- hw/ppc/spapr.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)