[Qemu-devel] [PATCH qemu] spapr: Drop unused parameters from fdt building helper

[Qemu-devel] [PATCH qemu] spapr: Drop unused parameters from fdt building helper

[Alexey Kardashevskiy]

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(-)

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);
 
-- 
2.17.1

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] spapr: Drop unused parameters from fdt building helper

[Greg Kurz]

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);
>  

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] spapr: Drop unused parameters from fdt building helper

[Philippe Mathieu-Daudé]

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);
>>  
> 
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] spapr: Drop unused parameters from fdt building helper

[Greg Kurz]

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);
> >>    
> > 
> >   

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] spapr: Drop unused parameters from fdt building helper

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]

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);
> > >>    
> > > 
> > >   
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]