lkml.org 
[lkml]   [2008]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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
    >




    \
     
     \ /
      Last update: 2008-04-29 22:57    [W:0.024 / U:0.432 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site