lkml.org 
[lkml]   [2008]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 1/4] x86: vSMP: Fix is_vsmp_box()
    On Fri, Mar 21, 2008 at 2:11 AM, Ingo Molnar <mingo@elte.hu> wrote:
    >
    > * Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
    >
    >
    > > >> /* Check if we are running on a ScaleMP vSMP box */
    > > >> - if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
    > > >> - (PCI_VENDOR_ID_SCALEMP || (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
    > > >> + if ((read_pci_config_16(0, 0x1f, 0, PCI_VENDOR_ID) ==
    > > >> + PCI_VENDOR_ID_SCALEMP) &&
    > > >> + (read_pci_config_16(0, 0x1f, 0, PCI_DEVICE_ID) ==
    > > >> + PCI_DEVICE_ID_SCALEMP_VSMP_CTL))
    > > >> vsmp = 1;
    > > >>
    > > >> return vsmp;
    > > >
    > > >why read two times
    > > >
    > >
    > > Well, the pci cfg space read happens just _once_ during the boot, as
    > > the result is cached in a static flag. The above code is better
    > > readable. So readability is better than micro-optimization here.
    >
    > i think the patch below results in even more readable and a bit smaller
    > code - because it's such a simple check?
    >
    > OTOH, you are right in general, for example in mmconf-fam10h_64.c's
    > get_fam10h_pci_mmconf_base() function, we do this:
    >
    > id = read_pci_config(bus, slot, 0, PCI_VENDOR_ID);
    >
    > vendor = id & 0xffff;
    > device = (id>>16) & 0xffff;
    > if (pci_probes[i].vendor == vendor &&
    > pci_probes[i].device == device) {
    >
    > here it would indeed be cleaner to simply do:
    >
    > vendor = read_pci_config_16(bus, slot, 0, PCI_VENDOR_ID);
    > device = read_pci_config_16(bus, slot, 0, PCI_DEVICE_ID);
    >
    > instead of open-coding the 16-bit splitting.
    >
    > Ingo
    >
    > ------------>
    > Subject: x86: vsmp fix x86 vsmp fix is vsmp box cleanup
    > From: Ingo Molnar <mingo@elte.hu>
    > Date: Fri Mar 21 09:55:06 CET 2008
    >
    > code got a bit smaller:
    >
    > arch/x86/kernel/vsmp_64.o:
    >
    > text data bss dec hex filename
    > 205 4 0 209 d1 vsmp_64.o.before
    > 181 4 0 185 b9 vsmp_64.o.after

    good evidence.

    YH


    \
     
     \ /
      Last update: 2008-03-21 10:21    [W:0.025 / U:0.028 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site