Re: [Qemu-ppc] [Qemu-devel] [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 <address@hidden> 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 <address@hidden>
> 
>> Signed-off-by: Alexey Kardashevskiy <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

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