lkml.org 
[lkml]   [2008]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [patch 1/4] x86: vSMP: Fix is_vsmp_box()

* 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

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/x86/kernel/vsmp_64.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

Index: linux-x86.q/arch/x86/kernel/vsmp_64.c
===================================================================
--- linux-x86.q.orig/arch/x86/kernel/vsmp_64.c
+++ linux-x86.q/arch/x86/kernel/vsmp_64.c
@@ -120,10 +120,8 @@ int is_vsmp_box(void)
return vsmp;

/* Check if we are running on a ScaleMP vSMP box */
- 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))
+ if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
+ (PCI_VENDOR_ID_SCALEMP | (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
vsmp = 1;

return vsmp;

\
 
 \ /
  Last update: 2008-03-21 10:15    [W:0.051 / U:0.648 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site