Messages in this thread | | | Date | Tue, 29 Apr 2008 16:57:20 -0400 | From | Andres Salomon <> | Subject | Re: [PATCH] x86: GEODE: cache results from geode_has_vsa2() and uninline |
| |
On Tue, 29 Apr 2008 13:35:12 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 29 Apr 2008 01:32:13 -0400 > Andres Salomon <dilinger@queued.net> wrote: > > > On Mon, 28 Apr 2008 20:06:51 -0700 > > Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > On Mon, 21 Apr 2008 17:02:30 -0400 Andres Salomon <dilinger@queued.net> wrote: > > > > > > > + if (!is_geode() || geode_has_vsa2()) > > > > > > geode_has_vsa2() is a fairly expensive-looking function and afacit only > > > needs to be evaluated once per boot. Perhaps we should cache it somewhere? > > > > > > > How about this? > > > > Looks sane. Although one wonders if it should be cached as one of the > standard x86 feature bit thingies which show up in /proc/cpuinfo's 'flags' > field. >
The VSA lives in a weird place between hardware and BIOS. I'm not really sure whether it's appropriate for it to be an x86_cap_flags (it hadn't occurred to me), but I think of it more as BIOS. Jordan, what do you think?
> > +static int has_vsa2 = -1; > > + > > +int geode_has_vsa2(void) > > +{ > > + if (has_vsa2 == -1) { > > + /* > > + * The VSA has virtual registers that we can query for a > > + * signature. > > + */ > > + outw(VSA_VR_UNLOCK, VSA_VRC_INDEX); > > + outw(VSA_VR_SIGNATURE, VSA_VRC_INDEX); > > + > > + has_vsa2 = (inw(VSA_VRC_DATA) == VSA_SIG); > > + } > > + > > + return has_vsa2; > > +} > > +EXPORT_SYMBOL_GPL(geode_has_vsa2); > > nit: > > --- a/arch/x86/kernel/geode_32.c > +++ a/arch/x86/kernel/geode_32.c > @@ -161,10 +161,10 @@ void geode_gpio_setup_event(unsigned int > } > EXPORT_SYMBOL_GPL(geode_gpio_setup_event); > > -static int has_vsa2 = -1; > - > int geode_has_vsa2(void) > { > + static int has_vsa2 = -1; > +
Looks good.
Acked-by: Andres Salomon <dilinger@debian.org>
> if (has_vsa2 == -1) { > /* > * The VSA has virtual registers that we can query for a >
| |