I was about to ack this, but it fails to build, see below. I also added a few smaller comments to the review, but the build issue is the only really important one.
On Sun, Nov 30, 2008 at 10:07:55PM +0100, Stefan Reinauer wrote:
+#ifndef DARWIN +#include <sys/io.h> +#else +#include <DirectIO/darwinio.h> +#endif
Maybe a small paragraph with build instructions and URL for DirectIO etc. in the README/manpage would be good (that's for another patch though).
#define PCI_DEVICE_ID_INTEL_82845 0x1a30 #define PCI_DEVICE_ID_INTEL_82945P 0x2770 #define PCI_DEVICE_ID_INTEL_82945GM 0x27a0 +#define PCI_DEVICE_ID_INTEL_PM965 0x2a00 +#define PCI_DEVICE_ID_INTEL_82975X 0x277c
These two use spaces, not TABs, and are thus not properly aligned, please change to TABs in the commit.
for (i = 0; i < size; i += 4) { +#ifdef DARWIN
if (i==0x14) {
^ ^ add spaces here, please.
/* Reading this register will hang a macbook pro */printf("pmbase+0x%04x: 0xXXXXXXXX\n", i);
Any idea why? Hardware problem or OS problem? Does Linux on Apple hardware have the same issue? If yes, the "#ifdef DARWIN" check may not be correct/complete.
+void unmap_physical(void *virt_addr, int len) +{
munmap((void *)rcba, size);+}
This part fails to build, as 'rcba' is never defined. I guess 'virt_addr' is the correct thing to use here (?) If yes, the (void *) cast is not needed, too, I think.
Also, s/size/len/, otherwise it won't build.
Index: Makefile
--- Makefile (revision 3783) +++ Makefile (working copy) @@ -29,6 +29,13 @@
OBJS = inteltool.o cpu.o gpio.o rootcmplx.o powermgt.o memory.o pcie.o
+OS_ARCH = $(shell uname) +ifeq ($(OS_ARCH), Darwin) +CFLAGS += -DDARWIN -I/usr/local/include +LDFLAGS = -framework IOKit -framework DirectIO -L/usr/local/lib -lpci -lz
Maybe /usr/local should be easily configurable (adding it as variable, or reusing PREFIX or so)?
+# OBJS += darwinio.o +endif
all: pciutils dep $(PROGRAM)
$(PROGRAM): $(OBJS) @@ -41,10 +48,10 @@ rm -f .dependencies
dep:
- @$(CC) -MM *.c > .dependencies
- @$(CC) $(CFLAGS) -MM *.c > .dependencies
pciutils:
- @echo; echo -n "Checking for pciutils and zlib... "
- @echo; /bin/echo -n "Checking for pciutils and zlib... "
Why this change? If it's needed, the "@echo" should also have the full path then?
Uwe.