Re: [Qemu-ppc] [PATCH] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c

[Thomas Huth]

On 2018-10-05 06:25, David Gibson wrote:
> On Thu, Oct 04, 2018 at 12:07:01PM +0200, Thomas Huth wrote:
>> The spapr-rng device is suboptimal when compared to virtio-rng, so
>> users might want to disable it in their builds. Thus let's introduce
>> a proper CONFIG switch to allow us to compile QEMU without this device.
>>
>> Signed-off-by: Thomas Huth <address@hidden>
> 
> Uh, sure, I guess so.

Yes, we definitely want this for the QEMU in RHEL :-)

>> diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
>> index d2acd61..dec8434 100644
>> --- a/hw/ppc/spapr_rng.c
>> +++ b/hw/ppc/spapr_rng.c
>> @@ -27,6 +27,7 @@
>>  #include "sysemu/rng.h"
>>  #include "hw/ppc/spapr.h"
>>  #include "kvm_ppc.h"
>> +#include "spapr_rng.h"
>>  
>>  #define SPAPR_RNG(obj) \
>>      OBJECT_CHECK(sPAPRRngState, (obj), TYPE_SPAPR_RNG)
>> @@ -132,29 +133,6 @@ static void spapr_rng_realize(DeviceState *dev, Error 
>> **errp)
>>      }
>>  }
>>  
>> -int spapr_rng_populate_dt(void *fdt)
>> -{
>> -    int node;
>> -    int ret;
>> -
>> -    node = qemu_fdt_add_subnode(fdt, "/ibm,platform-facilities");
>> -    if (node <= 0) {
>> -        return -1;
>> -    }
>> -    ret = fdt_setprop_string(fdt, node, "device_type",
>> -                             "ibm,platform-facilities");
>> -    ret |= fdt_setprop_cell(fdt, node, "#address-cells", 0x1);
>> -    ret |= fdt_setprop_cell(fdt, node, "#size-cells", 0x0);
>> -
>> -    node = fdt_add_subnode(fdt, node, "ibm,random-v1");
>> -    if (node <= 0) {
>> -        return -1;
>> -    }
>> -    ret |= fdt_setprop_string(fdt, node, "compatible", "ibm,random");
>> -
>> -    return ret ? -1 : 0;
>> -}
>> -
> 
> Moving this to an inline doesn't seem right to me though - it's a more
> complex function that we usually want in a .h inline, and I don't
> really see a good reason for it to be there (rather than an #ifdeffed
> stub).

An #ifdef is not possible here - the CONFIG switches for the targets are
*not* turned into pre-processor macros, only the CONFIG switches for the
host settings. So putting this function as static inline into a separate
header seems to be the best option to me right now. Alternatively, I
could also put it directly into spapr.c directly, but that file is
already very big... well, I don't mind, let me know what you prefer.

 Thomas

signature.asc
Description: OpenPGP digital signature