[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter

[Gleb Natapov <gleb@redhat.com>]

On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> There is no need starting with the special value for hpet_cfg.count.
> >> Either Seabios is aware of the new firmware interface and properly
> >> interprets the counter or it simply ignores it anyway.
> >>
> > I want seabios to be able to distinguish between old qemu and new one.
> 
> I see now. But isn't it a good chance to introduce a proper generic
> interface for exploring supported fw-cfg keys?
> 
Having such interface would be nice. Pity we haven't introduced it from
the start. If we do it now seabios will have to find out somehow that
qemu support such interface. Chicken and egg ;)

> Jan
> 
> > Hence special value. I used it incorrectly in may v2 seabios patch. Will
> > resend asap. Will teach me to not change logic at the last minute :( I
> > removed "valid" field between v1 and v2 of the patches and introduces
> > special value for count instead. As a result I made one bug in qemu and
> > one is seabios. Heh.
> > 
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  hw/hpet.c |    7 +------
> >>  1 files changed, 1 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/hw/hpet.c b/hw/hpet.c
> >> index d5c406c..ed4e995 100644
> >> --- a/hw/hpet.c
> >> +++ b/hw/hpet.c
> >> @@ -74,7 +74,7 @@ typedef struct HPETState {
> >>      uint8_t  hpet_id;           /* instance id */
> >>  } HPETState;
> >>  
> >> -struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
> >> +struct hpet_fw_config hpet_cfg;
> >>  
> >>  static uint32_t hpet_in_legacy_mode(HPETState *s)
> >>  {
> >> @@ -682,11 +682,6 @@ static int hpet_init(SysBusDevice *dev)
> >>      int i, iomemtype;
> >>      HPETTimer *timer;
> >>  
> >> -    if (hpet_cfg.count == UINT8_MAX) {
> >> -        /* first instance */
> >> -        hpet_cfg.count = 0;
> >> -    }
> >> -
> >>      if (hpet_cfg.count == 8) {
> >>          fprintf(stderr, "Only 8 instances of HPET is allowed\n");
> >>          return -1;
> >> -- 
> >> 1.6.0.2
> > 
> > --
> > 			Gleb.
> 
> 



--
			Gleb.