Re: [Qemu-devel] [PATCH 05/15] piix: create the HPET and RTC through composition

[Jan Kiszka]

On 2012-01-26 20:00, Anthony Liguori wrote:
> Signed-off-by: Anthony Liguori <address@hidden>
> ---
>  hw/hpet.c        |   38 +-------------------------
>  hw/hpet_emul.h   |   40 ++++++++++++++++++++++++++++
>  hw/mc146818rtc.c |   30 ++-------------------
>  hw/mc146818rtc.h |   27 +++++++++++++++++++
>  hw/pc.c          |   38 +++++----------------------
>  hw/piix_pci.c    |   76 ++++++++++++++++++++++++++++++++++++++++++++++-------
>  6 files changed, 145 insertions(+), 104 deletions(-)
> 
> diff --git a/hw/hpet.c b/hw/hpet.c
> index b6ace4e..c5b8b9e 100644
> --- a/hw/hpet.c
> +++ b/hw/hpet.c
> @@ -41,40 +41,6 @@
>  
>  #define HPET_MSI_SUPPORT        0
>  
> -struct HPETState;
> -typedef struct HPETTimer {  /* timers */
> -    uint8_t tn;             /*timer number*/
> -    QEMUTimer *qemu_timer;
> -    struct HPETState *state;
> -    /* Memory-mapped, software visible timer registers */
> -    uint64_t config;        /* configuration/cap */
> -    uint64_t cmp;           /* comparator */
> -    uint64_t fsb;           /* FSB route */
> -    /* Hidden register state */
> -    uint64_t period;        /* Last value written to comparator */
> -    uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 
> 32-bit
> -                             * mode. Next pop will be actual timer 
> expiration.
> -                             */
> -} HPETTimer;
> -
> -typedef struct HPETState {
> -    SysBusDevice busdev;
> -    MemoryRegion iomem;
> -    uint64_t hpet_offset;
> -    qemu_irq irqs[HPET_NUM_IRQ_ROUTES];
> -    uint32_t flags;
> -    uint8_t rtc_irq_level;
> -    uint8_t num_timers;
> -    HPETTimer timer[HPET_MAX_TIMERS];
> -
> -    /* Memory-mapped, software visible registers */
> -    uint64_t capability;        /* capabilities */
> -    uint64_t config;            /* configuration */
> -    uint64_t isr;               /* interrupt status reg */
> -    uint64_t hpet_counter;      /* main counter */
> -    uint8_t  hpet_id;           /* instance id */
> -} HPETState;
> -

Both structs are private and should remain so, same for similar patches
in this series. Does your composition concept requires publicizing them?
If yes, can't it be fixed. Would be a step backward if not.

Also note that the HPET is not a part of the PIIX, so composition is
wrong here. The RTC is again.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux