Re: [Qemu-devel] [PATCH v6 3/5] PC: use qdev_xx to create hpet instead of sysbus_create_xx

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Oct 10, 2013 at 03:56:17PM +0800, Liu Ping Fan wrote:
> sysbus_create_xx func does not allow us to set a device's extra
> properties.  While hpet need to set its compat property before
> initialization, so we abandon the wrapper function, and spread
> its logic "inline"
> 
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>


I would merge patches 3,4,5 together.
It's generally not a good idea to change
same line of code in multiple patches in a patchset,
makes review harder instead of easier.

> ---
>  hw/i386/pc.c    | 11 +++++++++--
>  hw/timer/hpet.c |  4 +---
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0c313fe..f2b7b6c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1246,9 +1246,16 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
>       * when the HPET wants to take over. Thus we have to disable the latter.
>       */
>      if (!no_hpet && (!kvm_irqchip_in_kernel() || kvm_has_pit_state2())) {
> -        hpet = sysbus_try_create_simple("hpet", HPET_BASE, NULL);
> -
> +        /* In order to set property, here not using sysbus_try_create_simple */
> +        hpet = qdev_try_create(NULL, "hpet");
>          if (hpet) {
> +            /* tmp fix. For compat, hard code to IRQ2 until we have correct
> +             * compat property and differentiate pc-iix with pc-q35
> +             */
> +            qdev_prop_set_uint32(hpet, "intcap", 0x4);
> +            qdev_init_nofail(hpet);
> +            sysbus_mmio_map(SYS_BUS_DEVICE(hpet), 0, HPET_BASE);
> +
>              for (i = 0; i < GSI_NUM_PINS; i++) {
>                  sysbus_connect_irq(SYS_BUS_DEVICE(hpet), i, gsi[i]);
>              }
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index 5b11be4..69ce587 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -43,8 +43,6 @@
>  
>  #define HPET_MSI_SUPPORT        0
>  
> -/* Will fix: intcap is set by board, and should be 0 if nobody sets. */
> -#define HPET_TN_INT_CAP_DEFAULT 0x4ULL
>  
>  #define TYPE_HPET "hpet"
>  #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
> @@ -758,7 +756,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
>  static Property hpet_device_properties[] = {
>      DEFINE_PROP_UINT8("timers", HPETState, num_timers, HPET_MIN_TIMERS),
>      DEFINE_PROP_BIT("msi", HPETState, flags, HPET_MSI_SUPPORT, false),
> -    DEFINE_PROP_UINT32("intcap", HPETState, intcap, HPET_TN_INT_CAP_DEFAULT),
> +    DEFINE_PROP_UINT32("intcap", HPETState, intcap, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -- 
> 1.8.1.4