2004-11-03 17:28:14

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH] fix HPET time_interpolator registration

Fixup after mid-air collision between Christoph adding time_interpolator.mask,
and me removing a static time_interpolator struct from hpet.c. This causes
a boot-time BUG() on boxes that use the hpet driver.


Attachments:
(No filename) (208.00 B)
hpet-mask.patch (619.00 B)
Download all attachments

2004-11-04 02:59:02

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] fix HPET time_interpolator registration

Bjorn Helgaas <[email protected]> wrote:
>
> Fixup after mid-air collision between Christoph adding time_interpolator.mask,
> and me removing a static time_interpolator struct from hpet.
>
> Signed-off-by: Bjorn Helgaas <[email protected]>
>
> ===== drivers/char/hpet.c 1.14 vs edited =====
> --- 1.14/drivers/char/hpet.c 2004-11-02 07:40:42 -07:00
> +++ edited/drivers/char/hpet.c 2004-11-03 10:05:26 -07:00
> @@ -712,6 +712,7 @@
> ti->addr = &hpetp->hp_hpet->hpet_mc;
> ti->frequency = hpet_time_div(hpets->hp_period);
> ti->drift = ti->frequency * HPET_DRIFT / 1000000;
> + ti->mask = 0xffffffffffffffffLL;
>
> hpetp->hp_interpolator = ti;
> register_time_interpolator(ti);
>

ti->mask is u64, and on some architectures u64 is `long'. Compilers might
whine about this. I'll make it

ti->mask = -1;

which just works.

2004-11-04 03:53:12

by Christoph Lameter

[permalink] [raw]
Subject: Re: [PATCH] fix HPET time_interpolator registration

On Wed, 3 Nov 2004, Andrew Morton wrote:

> Bjorn Helgaas <[email protected]> wrote:
> >
> > Fixup after mid-air collision between Christoph adding time_interpolator.mask,
> > and me removing a static time_interpolator struct from hpet.
> >
> > Signed-off-by: Bjorn Helgaas <[email protected]>
> >
> > ===== drivers/char/hpet.c 1.14 vs edited =====
> > --- 1.14/drivers/char/hpet.c 2004-11-02 07:40:42 -07:00
> > +++ edited/drivers/char/hpet.c 2004-11-03 10:05:26 -07:00
> > @@ -712,6 +712,7 @@
> > ti->addr = &hpetp->hp_hpet->hpet_mc;
> > ti->frequency = hpet_time_div(hpets->hp_period);
> > ti->drift = ti->frequency * HPET_DRIFT / 1000000;
> > + ti->mask = 0xffffffffffffffffLL;
> >
> > hpetp->hp_interpolator = ti;
> > register_time_interpolator(ti);
> >
>
> ti->mask is u64, and on some architectures u64 is `long'. Compilers might
> whine about this. I'll make it
>
> ti->mask = -1;
>
> which just works.

Hmmm... How do you then specify a 64 bit mask without running into issues
with the compilers?

2004-11-04 04:48:15

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] fix HPET time_interpolator registration

Christoph Lameter <[email protected]> wrote:
>
> On Wed, 3 Nov 2004, Andrew Morton wrote:
>
> > Bjorn Helgaas <[email protected]> wrote:
> > >
> > > Fixup after mid-air collision between Christoph adding time_interpolator.mask,
> > > and me removing a static time_interpolator struct from hpet.
> > >
> > > Signed-off-by: Bjorn Helgaas <[email protected]>
> > >
> > > ===== drivers/char/hpet.c 1.14 vs edited =====
> > > --- 1.14/drivers/char/hpet.c 2004-11-02 07:40:42 -07:00
> > > +++ edited/drivers/char/hpet.c 2004-11-03 10:05:26 -07:00
> > > @@ -712,6 +712,7 @@
> > > ti->addr = &hpetp->hp_hpet->hpet_mc;
> > > ti->frequency = hpet_time_div(hpets->hp_period);
> > > ti->drift = ti->frequency * HPET_DRIFT / 1000000;
> > > + ti->mask = 0xffffffffffffffffLL;
> > >
> > > hpetp->hp_interpolator = ti;
> > > register_time_interpolator(ti);
> > >
> >
> > ti->mask is u64, and on some architectures u64 is `long'. Compilers might
> > whine about this. I'll make it
> >
> > ti->mask = -1;
> >
> > which just works.
>
> Hmmm... How do you then specify a 64 bit mask without running into issues
> with the compilers?

Well with 0xffffffff[ffffffff] it's easy: use -1 and sign extension.

The only problem I can see is if you want to propagate a bit pattern across
the scalar but you don't know its size. Say 0x5a5a5a5a versus
0x5a5a5a5a5a5a5a5a. But nobody ever wants to do that.