[PATCH] Sort generic PCI fixups after specific ones

[PATCH] Sort generic PCI fixups after specific ones

[Matthew Wilcox]

The recent change that allowed PCI fixups to be declared everywhere
broke IDE on PA-RISC by making the generic IDE fixup be applied before
the PA-RISC specific one.  This patch fixes that by sorting generic fixups
after the specific ones.  It also obeys the 80-column limit and reduces
the amount of grotty macro code.

I'd like to thank Joel Soete for his work tracking down the source of
this problem.

Index: linux-2.6/drivers/pci/quirks.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/pci/quirks.c,v
retrieving revision 1.16
diff -u -p -r1.16 quirks.c
--- linux-2.6/drivers/pci/quirks.c	13 Sep 2004 15:23:21 -0000	1.16
+++ linux-2.6/drivers/pci/quirks.c	22 Sep 2004 21:38:17 -0000
@@ -543,7 +543,7 @@ static void __devinit quirk_cardbus_lega
 		return;
 	pci_write_config_dword(dev, PCI_CB_LEGACY_MODE_BASE, 0);
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID,		PCI_ANY_ID,			quirk_cardbus_legacy );
+DECLARE_PCI_FIXUP_FINAL_ALL(quirk_cardbus_legacy);
 
 /*
  * Following the PCI ordering rules is optional on the AMD762. I'm not
@@ -661,7 +661,7 @@ static void __devinit quirk_ide_bases(st
        printk(KERN_INFO "PCI: Ignoring BAR%d-%d of IDE controller %s\n",
               first_bar, last_bar, pci_name(dev));
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID,             PCI_ANY_ID,                     quirk_ide_bases );
+DECLARE_PCI_FIXUP_HEADER_ALL(quirk_ide_bases);
 
 /*
  *	Ensure C0 rev restreaming is off. This is normally done by
Index: linux-2.6/include/asm-generic/vmlinux.lds.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-generic/vmlinux.lds.h,v
retrieving revision 1.4
diff -u -p -r1.4 vmlinux.lds.h
--- linux-2.6/include/asm-generic/vmlinux.lds.h	13 Sep 2004 15:24:02 -0000	1.4
+++ linux-2.6/include/asm-generic/vmlinux.lds.h	22 Sep 2004 21:38:23 -0000
@@ -20,9 +20,11 @@
 	.pci_fixup        : AT(ADDR(.pci_fixup) - LOAD_OFFSET) {	\
 		VMLINUX_SYMBOL(__start_pci_fixups_header) = .;		\
 		*(.pci_fixup_header)					\
+		*(.pci_fixup_header_all)				\
 		VMLINUX_SYMBOL(__end_pci_fixups_header) = .;		\
 		VMLINUX_SYMBOL(__start_pci_fixups_final) = .;		\
 		*(.pci_fixup_final)					\
+		*(.pci_fixup_final_all)					\
 		VMLINUX_SYMBOL(__end_pci_fixups_final) = .;		\
 	}								\
 									\
Index: linux-2.6/include/linux/pci.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/linux/pci.h,v
retrieving revision 1.18
diff -u -p -r1.18 pci.h
--- linux-2.6/include/linux/pci.h	13 Sep 2004 15:24:12 -0000	1.18
+++ linux-2.6/include/linux/pci.h	22 Sep 2004 21:38:25 -0000
@@ -1011,15 +1011,22 @@ enum pci_fixup_pass {
 };
 
 /* Anonymous variables would be nice... */
-#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)					\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_header"))) = {				\
-		vendor, device, hook };
-
-#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)				\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_final"))) = {				\
+#define DECLARE_PCI_FIXUP_SECTION(section, name, vendor, device, hook)	\
+	static struct pci_fixup __pci_fixup_##name			\
+	__attribute_used__ __attribute__((__section__( #section ))) = {	\
 		vendor, device, hook };
+#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)			\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header,			\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_HEADER_ALL(hook)				\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header_all,		\
+			ALL_DEVICES##hook, PCI_ANY_ID, PCI_ANY_ID, hook)
+#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)			\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final,			\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_FINAL_ALL(hook)				\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final_all,			\
+			ALL_DEVICES##hook, PCI_ANY_ID, PCI_ANY_ID, hook)
 
 void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
 

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] Sort generic PCI fixups after specific ones

[Matthew Wilcox]

On Sat, Oct 02, 2004 at 01:48:17AM +0400, Ivan Kokshaysky wrote:
> On Thu, Sep 30, 2004 at 06:41:55PM +0100, Matthew Wilcox wrote:
> > Allow prioritising PCI fixups.  "How it works" is covered in the comment
> > in pci.h.  The patch to superio.c may well only apply with fuzz to the
> > current Linux tree; I include it only to show an example.
> 
> No, you missed my point.
> What we need is yet another PCI fixup *pass*, not prioritizing fixups
> inside *one* pass - see appended patch (compile tested only).

Boot tested.  Works fine for my problem child.  Greg, can you apply
Ivan's patch, please?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] Sort generic PCI fixups after specific ones

[Matthew Wilcox]

On Sat, Oct 02, 2004 at 01:48:17AM +0400, Ivan Kokshaysky wrote:
> On Thu, Sep 30, 2004 at 06:41:55PM +0100, Matthew Wilcox wrote:
> > Allow prioritising PCI fixups.  "How it works" is covered in the comment
> > in pci.h.  The patch to superio.c may well only apply with fuzz to the
> > current Linux tree; I include it only to show an example.
> 
> No, you missed my point.
> What we need is yet another PCI fixup *pass*, not prioritizing fixups
> inside *one* pass - see appended patch (compile tested only).

Boot tested.  Works fine for my problem child.  Greg, can you apply
Ivan's patch, please?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] Sort generic PCI fixups after specific ones

[Ivan Kokshaysky]

On Thu, Sep 30, 2004 at 06:41:55PM +0100, Matthew Wilcox wrote:
> Allow prioritising PCI fixups.  "How it works" is covered in the comment
> in pci.h.  The patch to superio.c may well only apply with fuzz to the
> current Linux tree; I include it only to show an example.

No, you missed my point.
What we need is yet another PCI fixup *pass*, not prioritizing fixups
inside *one* pass - see appended patch (compile tested only).
Speaking of IDE: generally, you cannot switch PCI IDE controller
from "compatible" to native mode in the "fixup header" pass, as at this
point PCI BARs are *already* probed, and in compatible mode a lot of IDE
controllers have some bogus (often read-only) values in the BARs 0-3.
That's why quirk_ide_bases() fixup exists in the first place.
So that parisc superio fix works by mere luck - probably because IDE
BARs 0-3 are readable/writable even in legacy mode on this particular
controller.

Ivan.

--- 2.6/include/asm-generic/vmlinux.lds.h	Mon Sep 27 14:38:14 2004
+++ linux/include/asm-generic/vmlinux.lds.h	Fri Oct  1 15:24:30 2004
@@ -18,6 +18,9 @@
 									\
 	/* PCI quirks */						\
 	.pci_fixup        : AT(ADDR(.pci_fixup) - LOAD_OFFSET) {	\
+		VMLINUX_SYMBOL(__start_pci_fixups_early) = .;		\
+		*(.pci_fixup_early)					\
+		VMLINUX_SYMBOL(__end_pci_fixups_early) = .;		\
 		VMLINUX_SYMBOL(__start_pci_fixups_header) = .;		\
 		*(.pci_fixup_header)					\
 		VMLINUX_SYMBOL(__end_pci_fixups_header) = .;		\
--- 2.6/include/linux/pci.h	Mon Sep 27 14:38:23 2004
+++ linux/include/linux/pci.h	Fri Oct  1 16:16:51 2004
@@ -1006,20 +1006,29 @@ struct pci_fixup {
 };
 
 enum pci_fixup_pass {
-	pci_fixup_header,	/* Called immediately after reading configuration header */
+	pci_fixup_early,	/* Called immediately after reading
+				   device/vendor IDs and class code */
+	pci_fixup_header,	/* Called after reading the entire
+				   configuration header (including BARs) */
 	pci_fixup_final,	/* Final phase of device fixups */
 };
 
 /* Anonymous variables would be nice... */
-#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)					\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_header"))) = {				\
-		vendor, device, hook };
+#define DECLARE_PCI_FIXUP_SECTION(section, name, vendor, device, hook)	\
+	static struct pci_fixup __pci_fixup_##name __attribute_used__	\
+	__attribute__((__section__( #section ))) = { vendor, device, hook };
 
-#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)				\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_final"))) = {				\
-		vendor, device, hook };
+#define DECLARE_PCI_FIXUP_EARLY(vendor, device, hook)			\
+	 DECLARE_PCI_FIXUP_SECTION(.pci_fixup_early,			\
+			vendor##device##hook, vendor, device, hook)
+
+#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)			\
+	 DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header,			\
+			vendor##device##hook, vendor, device, hook)
+
+#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)			\
+	 DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final,			\
+			vendor##device##hook, vendor, device, hook)
 
 void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
 
--- 2.6/drivers/pci/quirks.c	Mon Sep 27 14:37:45 2004
+++ linux/drivers/pci/quirks.c	Fri Oct  1 16:24:16 2004
@@ -999,6 +999,8 @@ static void pci_do_fixups(struct pci_dev
 	}
 }
 
+extern struct pci_fixup __start_pci_fixups_early[];
+extern struct pci_fixup __end_pci_fixups_early[];
 extern struct pci_fixup __start_pci_fixups_header[];
 extern struct pci_fixup __end_pci_fixups_header[];
 extern struct pci_fixup __start_pci_fixups_final[];
@@ -1009,6 +1011,11 @@ void pci_fixup_device(enum pci_fixup_pas
 	struct pci_fixup *start, *end;
 
 	switch(pass) {
+	case pci_fixup_early:
+		start = __start_pci_fixups_early;
+		end = __end_pci_fixups_early;
+		break;
+
 	case pci_fixup_header:
 		start = __start_pci_fixups_header;
 		end = __end_pci_fixups_header;
--- 2.6/drivers/pci/probe.c	Mon Sep 27 14:37:45 2004
+++ linux/drivers/pci/probe.c	Fri Oct  1 15:30:00 2004
@@ -475,6 +475,9 @@ static int pci_setup_device(struct pci_d
 	/* "Unknown power state" */
 	dev->current_state = 4;
 
+	/* Early fixups, before probing the BARs */
+	pci_fixup_device(pci_fixup_early, dev);
+
 	switch (dev->hdr_type) {		    /* header type */
 	case PCI_HEADER_TYPE_NORMAL:		    /* standard header */
 		if (class == PCI_CLASS_BRIDGE_PCI)
--- 2.6/drivers/parisc/superio.c	Mon Sep 27 14:37:45 2004
+++ linux/drivers/parisc/superio.c	Fri Oct  1 15:32:20 2004
@@ -484,7 +484,7 @@ void superio_fixup_pci(struct pci_dev *p
 	pci_read_config_byte(pdev, PCI_CLASS_PROG, &prog);
 	printk("PCI: Enabled native mode for NS87415 (pif=0x%x)\n", prog);
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87415, superio_fixup_pci);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87415, superio_fixup_pci);
 
 /* Because of a defect in Super I/O, all reads of the PCI DMA status 
  * registers, IDE status register and the IDE select register need to be 

Re: [PATCH] Sort generic PCI fixups after specific ones

[Ivan Kokshaysky]

On Thu, Sep 30, 2004 at 06:41:55PM +0100, Matthew Wilcox wrote:
> Allow prioritising PCI fixups.  "How it works" is covered in the comment
> in pci.h.  The patch to superio.c may well only apply with fuzz to the
> current Linux tree; I include it only to show an example.

No, you missed my point.
What we need is yet another PCI fixup *pass*, not prioritizing fixups
inside *one* pass - see appended patch (compile tested only).
Speaking of IDE: generally, you cannot switch PCI IDE controller
from "compatible" to native mode in the "fixup header" pass, as at this
point PCI BARs are *already* probed, and in compatible mode a lot of IDE
controllers have some bogus (often read-only) values in the BARs 0-3.
That's why quirk_ide_bases() fixup exists in the first place.
So that parisc superio fix works by mere luck - probably because IDE
BARs 0-3 are readable/writable even in legacy mode on this particular
controller.

Ivan.

--- 2.6/include/asm-generic/vmlinux.lds.h	Mon Sep 27 14:38:14 2004
+++ linux/include/asm-generic/vmlinux.lds.h	Fri Oct  1 15:24:30 2004
@@ -18,6 +18,9 @@
 									\
 	/* PCI quirks */						\
 	.pci_fixup        : AT(ADDR(.pci_fixup) - LOAD_OFFSET) {	\
+		VMLINUX_SYMBOL(__start_pci_fixups_early) = .;		\
+		*(.pci_fixup_early)					\
+		VMLINUX_SYMBOL(__end_pci_fixups_early) = .;		\
 		VMLINUX_SYMBOL(__start_pci_fixups_header) = .;		\
 		*(.pci_fixup_header)					\
 		VMLINUX_SYMBOL(__end_pci_fixups_header) = .;		\
--- 2.6/include/linux/pci.h	Mon Sep 27 14:38:23 2004
+++ linux/include/linux/pci.h	Fri Oct  1 16:16:51 2004
@@ -1006,20 +1006,29 @@ struct pci_fixup {
 };
 
 enum pci_fixup_pass {
-	pci_fixup_header,	/* Called immediately after reading configuration header */
+	pci_fixup_early,	/* Called immediately after reading
+				   device/vendor IDs and class code */
+	pci_fixup_header,	/* Called after reading the entire
+				   configuration header (including BARs) */
 	pci_fixup_final,	/* Final phase of device fixups */
 };
 
 /* Anonymous variables would be nice... */
-#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)					\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_header"))) = {				\
-		vendor, device, hook };
+#define DECLARE_PCI_FIXUP_SECTION(section, name, vendor, device, hook)	\
+	static struct pci_fixup __pci_fixup_##name __attribute_used__	\
+	__attribute__((__section__( #section ))) = { vendor, device, hook };
 
-#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)				\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_final"))) = {				\
-		vendor, device, hook };
+#define DECLARE_PCI_FIXUP_EARLY(vendor, device, hook)			\
+	 DECLARE_PCI_FIXUP_SECTION(.pci_fixup_early,			\
+			vendor##device##hook, vendor, device, hook)
+
+#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)			\
+	 DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header,			\
+			vendor##device##hook, vendor, device, hook)
+
+#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)			\
+	 DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final,			\
+			vendor##device##hook, vendor, device, hook)
 
 void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
 
--- 2.6/drivers/pci/quirks.c	Mon Sep 27 14:37:45 2004
+++ linux/drivers/pci/quirks.c	Fri Oct  1 16:24:16 2004
@@ -999,6 +999,8 @@ static void pci_do_fixups(struct pci_dev
 	}
 }
 
+extern struct pci_fixup __start_pci_fixups_early[];
+extern struct pci_fixup __end_pci_fixups_early[];
 extern struct pci_fixup __start_pci_fixups_header[];
 extern struct pci_fixup __end_pci_fixups_header[];
 extern struct pci_fixup __start_pci_fixups_final[];
@@ -1009,6 +1011,11 @@ void pci_fixup_device(enum pci_fixup_pas
 	struct pci_fixup *start, *end;
 
 	switch(pass) {
+	case pci_fixup_early:
+		start = __start_pci_fixups_early;
+		end = __end_pci_fixups_early;
+		break;
+
 	case pci_fixup_header:
 		start = __start_pci_fixups_header;
 		end = __end_pci_fixups_header;
--- 2.6/drivers/pci/probe.c	Mon Sep 27 14:37:45 2004
+++ linux/drivers/pci/probe.c	Fri Oct  1 15:30:00 2004
@@ -475,6 +475,9 @@ static int pci_setup_device(struct pci_d
 	/* "Unknown power state" */
 	dev->current_state = 4;
 
+	/* Early fixups, before probing the BARs */
+	pci_fixup_device(pci_fixup_early, dev);
+
 	switch (dev->hdr_type) {		    /* header type */
 	case PCI_HEADER_TYPE_NORMAL:		    /* standard header */
 		if (class == PCI_CLASS_BRIDGE_PCI)
--- 2.6/drivers/parisc/superio.c	Mon Sep 27 14:37:45 2004
+++ linux/drivers/parisc/superio.c	Fri Oct  1 15:32:20 2004
@@ -484,7 +484,7 @@ void superio_fixup_pci(struct pci_dev *p
 	pci_read_config_byte(pdev, PCI_CLASS_PROG, &prog);
 	printk("PCI: Enabled native mode for NS87415 (pif=0x%x)\n", prog);
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87415, superio_fixup_pci);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87415, superio_fixup_pci);
 
 /* Because of a defect in Super I/O, all reads of the PCI DMA status 
  * registers, IDE status register and the IDE select register need to be 

Re: [PATCH] Sort generic PCI fixups after specific ones

[Matthew Wilcox]

On Fri, Sep 24, 2004 at 02:33:57AM +0400, Ivan Kokshaysky wrote:
> I believe we do need third level of fixups, specifically for devices like
> IDE which can change the PCI header contents (class code, BAR layout etc.)
> depending on some magic bits in their registers.
> Such "early" or "pre-header" fixups should be called right after the device
> discovery, before probing the BARs. Apparently pci_setup_device()
> is a proper place for that.

OK, fine.

Allow prioritising PCI fixups.  "How it works" is covered in the comment
in pci.h.  The patch to superio.c may well only apply with fuzz to the
current Linux tree; I include it only to show an example.

Index: linux-2.6/drivers/parisc/superio.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/parisc/superio.c,v
retrieving revision 1.11
diff -u -p -r1.11 superio.c
--- linux-2.6/drivers/parisc/superio.c	28 Sep 2004 17:07:01 -0000	1.11
+++ linux-2.6/drivers/parisc/superio.c	30 Sep 2004 17:36:34 -0000
@@ -481,7 +481,7 @@ void superio_fixup_pci(struct pci_dev *p
 	pci_read_config_byte(pdev, PCI_CLASS_PROG, &prog);
 	printk("PCI: Enabled native mode for NS87415 (pif=0x%x)\n", prog);
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87415, superio_fixup_pci);
+DECLARE_PCI_FIXUP_HEADER_PRI(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87415, superio_fixup_pci, 1);
 
 
 static int __devinit superio_probe(struct pci_dev *dev, const struct pci_device_id *id)
Index: linux-2.6/include/asm-generic/vmlinux.lds.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-generic/vmlinux.lds.h,v
retrieving revision 1.4
diff -u -p -r1.4 vmlinux.lds.h
--- linux-2.6/include/asm-generic/vmlinux.lds.h	13 Sep 2004 15:24:02 -0000	1.4
+++ linux-2.6/include/asm-generic/vmlinux.lds.h	30 Sep 2004 17:36:35 -0000
@@ -19,10 +19,14 @@
 	/* PCI quirks */						\
 	.pci_fixup        : AT(ADDR(.pci_fixup) - LOAD_OFFSET) {	\
 		VMLINUX_SYMBOL(__start_pci_fixups_header) = .;		\
-		*(.pci_fixup_header)					\
+		*(.pci_fixup_header1)					\
+		*(.pci_fixup_header2)					\
+		*(.pci_fixup_header3)					\
 		VMLINUX_SYMBOL(__end_pci_fixups_header) = .;		\
 		VMLINUX_SYMBOL(__start_pci_fixups_final) = .;		\
-		*(.pci_fixup_final)					\
+		*(.pci_fixup_final1)					\
+		*(.pci_fixup_final2)					\
+		*(.pci_fixup_final3)					\
 		VMLINUX_SYMBOL(__end_pci_fixups_final) = .;		\
 	}								\
 									\
Index: linux-2.6/include/linux/pci.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/linux/pci.h,v
retrieving revision 1.18
diff -u -p -r1.18 pci.h
--- linux-2.6/include/linux/pci.h	13 Sep 2004 15:24:12 -0000	1.18
+++ linux-2.6/include/linux/pci.h	30 Sep 2004 17:36:36 -0000
@@ -1010,16 +1010,29 @@ enum pci_fixup_pass {
 	pci_fixup_final,	/* Final phase of device fixups */
 };
 
-/* Anonymous variables would be nice... */
-#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)					\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_header"))) = {				\
-		vendor, device, hook };
-
-#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)				\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_final"))) = {				\
+/*
+ * Most fixups should be declared as DECLARE_PCI_FIXUP_HEADER() or
+ * DECLARE_PCI_FIXUP_FINAL().  If you need to ensure ordering, you can
+ * specify a priority.  Current available priorities are 1, 2 (the default)
+ * and 3.  If you need to add more priorities, see
+ * include/asm-generic/vmlinux.lds.h
+ */
+#define DECLARE_PCI_FIXUP_SECTION(section, name, vendor, device, hook)	\
+	static struct pci_fixup __pci_fixup_##name			\
+	__attribute_used__ __attribute__((__section__( #section ))) = {	\
 		vendor, device, hook };
+#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)			\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header2,			\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_HEADER_PRI(vendor, device, hook, pri)		\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header##pri,		\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)			\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final2,			\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_FINAL_PRI(vendor, device, hook, pri)		\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final##pri,		\
+			vendor##device##hook, vendor, device, hook)
 
 void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
 

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] Sort generic PCI fixups after specific ones

[Matthew Wilcox]

On Fri, Sep 24, 2004 at 02:33:57AM +0400, Ivan Kokshaysky wrote:
> I believe we do need third level of fixups, specifically for devices like
> IDE which can change the PCI header contents (class code, BAR layout etc.)
> depending on some magic bits in their registers.
> Such "early" or "pre-header" fixups should be called right after the device
> discovery, before probing the BARs. Apparently pci_setup_device()
> is a proper place for that.

OK, fine.

Allow prioritising PCI fixups.  "How it works" is covered in the comment
in pci.h.  The patch to superio.c may well only apply with fuzz to the
current Linux tree; I include it only to show an example.

Index: linux-2.6/drivers/parisc/superio.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/parisc/superio.c,v
retrieving revision 1.11
diff -u -p -r1.11 superio.c
--- linux-2.6/drivers/parisc/superio.c	28 Sep 2004 17:07:01 -0000	1.11
+++ linux-2.6/drivers/parisc/superio.c	30 Sep 2004 17:36:34 -0000
@@ -481,7 +481,7 @@ void superio_fixup_pci(struct pci_dev *p
 	pci_read_config_byte(pdev, PCI_CLASS_PROG, &prog);
 	printk("PCI: Enabled native mode for NS87415 (pif=0x%x)\n", prog);
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87415, superio_fixup_pci);
+DECLARE_PCI_FIXUP_HEADER_PRI(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87415, superio_fixup_pci, 1);
 
 
 static int __devinit superio_probe(struct pci_dev *dev, const struct pci_device_id *id)
Index: linux-2.6/include/asm-generic/vmlinux.lds.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-generic/vmlinux.lds.h,v
retrieving revision 1.4
diff -u -p -r1.4 vmlinux.lds.h
--- linux-2.6/include/asm-generic/vmlinux.lds.h	13 Sep 2004 15:24:02 -0000	1.4
+++ linux-2.6/include/asm-generic/vmlinux.lds.h	30 Sep 2004 17:36:35 -0000
@@ -19,10 +19,14 @@
 	/* PCI quirks */						\
 	.pci_fixup        : AT(ADDR(.pci_fixup) - LOAD_OFFSET) {	\
 		VMLINUX_SYMBOL(__start_pci_fixups_header) = .;		\
-		*(.pci_fixup_header)					\
+		*(.pci_fixup_header1)					\
+		*(.pci_fixup_header2)					\
+		*(.pci_fixup_header3)					\
 		VMLINUX_SYMBOL(__end_pci_fixups_header) = .;		\
 		VMLINUX_SYMBOL(__start_pci_fixups_final) = .;		\
-		*(.pci_fixup_final)					\
+		*(.pci_fixup_final1)					\
+		*(.pci_fixup_final2)					\
+		*(.pci_fixup_final3)					\
 		VMLINUX_SYMBOL(__end_pci_fixups_final) = .;		\
 	}								\
 									\
Index: linux-2.6/include/linux/pci.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/linux/pci.h,v
retrieving revision 1.18
diff -u -p -r1.18 pci.h
--- linux-2.6/include/linux/pci.h	13 Sep 2004 15:24:12 -0000	1.18
+++ linux-2.6/include/linux/pci.h	30 Sep 2004 17:36:36 -0000
@@ -1010,16 +1010,29 @@ enum pci_fixup_pass {
 	pci_fixup_final,	/* Final phase of device fixups */
 };
 
-/* Anonymous variables would be nice... */
-#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)					\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_header"))) = {				\
-		vendor, device, hook };
-
-#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)				\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_final"))) = {				\
+/*
+ * Most fixups should be declared as DECLARE_PCI_FIXUP_HEADER() or
+ * DECLARE_PCI_FIXUP_FINAL().  If you need to ensure ordering, you can
+ * specify a priority.  Current available priorities are 1, 2 (the default)
+ * and 3.  If you need to add more priorities, see
+ * include/asm-generic/vmlinux.lds.h
+ */
+#define DECLARE_PCI_FIXUP_SECTION(section, name, vendor, device, hook)	\
+	static struct pci_fixup __pci_fixup_##name			\
+	__attribute_used__ __attribute__((__section__( #section ))) = {	\
 		vendor, device, hook };
+#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)			\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header2,			\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_HEADER_PRI(vendor, device, hook, pri)		\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header##pri,		\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)			\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final2,			\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_FINAL_PRI(vendor, device, hook, pri)		\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final##pri,		\
+			vendor##device##hook, vendor, device, hook)
 
 void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
 

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] Sort generic PCI fixups after specific ones

[Ivan Kokshaysky]

On Thu, Sep 23, 2004 at 06:31:51PM +0100, Matthew Wilcox wrote:
> Not really.  There's two types of fixup (well, four if you multiply by
> the header vs later possibility).  There's the incredibly specific ("this
> device from this manufacturer forgets to set something properly") and
> the incredibly general ("if this is a cardbus / IDE device, then ...").
> This patch simply distinguishes between the two.  Obviously the general
> ones run after the specific ones -- there's specific devices that forget
> to set their class code, for example.

This is way not obvious. Example: some architecture requires to put
_all_ PCI IDE devices in native mode (obviously with a general fixup),
but what if some specific chips need additional "native mode only"
workarounds? Your logic won't work then.

I believe we do need third level of fixups, specifically for devices like
IDE which can change the PCI header contents (class code, BAR layout etc.)
depending on some magic bits in their registers.
Such "early" or "pre-header" fixups should be called right after the device
discovery, before probing the BARs. Apparently pci_setup_device()
is a proper place for that.

Ivan.

Re: [PATCH] Sort generic PCI fixups after specific ones

[Matthew Wilcox]

On Thu, Sep 23, 2004 at 10:20:38AM -0700, Greg KH wrote:
> On Wed, Sep 22, 2004 at 10:43:04PM +0100, Matthew Wilcox wrote:
> > The recent change that allowed PCI fixups to be declared everywhere
> > broke IDE on PA-RISC by making the generic IDE fixup be applied before
> > the PA-RISC specific one.  This patch fixes that by sorting generic fixups
> > after the specific ones.  It also obeys the 80-column limit and reduces
> > the amount of grotty macro code.
> 
> It looks like you are doing 2 different things here with this new macro.
> Having it run last, and leting the user not type the PCI_ANY_ID macro
> twice.  How about if you want to do a final final type pass, you mark it
> as such, and not try to hide it in this manner.

Not really.  There's two types of fixup (well, four if you multiply by
the header vs later possibility).  There's the incredibly specific ("this
device from this manufacturer forgets to set something properly") and
the incredibly general ("if this is a cardbus / IDE device, then ...").
This patch simply distinguishes between the two.  Obviously the general
ones run after the specific ones -- there's specific devices that forget
to set their class code, for example.

> And do we really want to call it "final final"?  What if we determine
> that we need a "final final final" pass?  Can't you fix this with the
> link order like was previously done?  I'd really prefer to not add
> another level.

I don't want to call it "final final" at all.  Did you miss the message
where I explained the problem with this being link order dependent?

> Oh, and cc:ing the pci maintainer might be nice next time :)

I already apologised to you on IRC for that yesterday.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] Sort generic PCI fixups after specific ones

[Greg KH]

On Wed, Sep 22, 2004 at 10:43:04PM +0100, Matthew Wilcox wrote:
> 
> The recent change that allowed PCI fixups to be declared everywhere
> broke IDE on PA-RISC by making the generic IDE fixup be applied before
> the PA-RISC specific one.  This patch fixes that by sorting generic fixups
> after the specific ones.  It also obeys the 80-column limit and reduces
> the amount of grotty macro code.
> 
> I'd like to thank Joel Soete for his work tracking down the source of
> this problem.
> 
> Index: linux-2.6/drivers/pci/quirks.c
> ===================================================================
> RCS file: /var/cvs/linux-2.6/drivers/pci/quirks.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 quirks.c
> --- linux-2.6/drivers/pci/quirks.c	13 Sep 2004 15:23:21 -0000	1.16
> +++ linux-2.6/drivers/pci/quirks.c	22 Sep 2004 21:38:17 -0000
> @@ -543,7 +543,7 @@ static void __devinit quirk_cardbus_lega
>  		return;
>  	pci_write_config_dword(dev, PCI_CB_LEGACY_MODE_BASE, 0);
>  }
> -DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID,		PCI_ANY_ID,			quirk_cardbus_legacy );
> +DECLARE_PCI_FIXUP_FINAL_ALL(quirk_cardbus_legacy);

It looks like you are doing 2 different things here with this new macro.
Having it run last, and leting the user not type the PCI_ANY_ID macro
twice.  How about if you want to do a final final type pass, you mark it
as such, and not try to hide it in this manner.

And do we really want to call it "final final"?  What if we determine
that we need a "final final final" pass?  Can't you fix this with the
link order like was previously done?  I'd really prefer to not add
another level.

Oh, and cc:ing the pci maintainer might be nice next time :)

thanks,

greg k-h

[PATCH] Sort generic PCI fixups after specific ones

[Matthew Wilcox]

The recent change that allowed PCI fixups to be declared everywhere
broke IDE on PA-RISC by making the generic IDE fixup be applied before
the PA-RISC specific one.  This patch fixes that by sorting generic fixups
after the specific ones.  It also obeys the 80-column limit and reduces
the amount of grotty macro code.

I'd like to thank Joel Soete for his work tracking down the source of
this problem.

Index: linux-2.6/drivers/pci/quirks.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/pci/quirks.c,v
retrieving revision 1.16
diff -u -p -r1.16 quirks.c
--- linux-2.6/drivers/pci/quirks.c	13 Sep 2004 15:23:21 -0000	1.16
+++ linux-2.6/drivers/pci/quirks.c	22 Sep 2004 21:38:17 -0000
@@ -543,7 +543,7 @@ static void __devinit quirk_cardbus_lega
 		return;
 	pci_write_config_dword(dev, PCI_CB_LEGACY_MODE_BASE, 0);
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID,		PCI_ANY_ID,			quirk_cardbus_legacy );
+DECLARE_PCI_FIXUP_FINAL_ALL(quirk_cardbus_legacy);
 
 /*
  * Following the PCI ordering rules is optional on the AMD762. I'm not
@@ -661,7 +661,7 @@ static void __devinit quirk_ide_bases(st
        printk(KERN_INFO "PCI: Ignoring BAR%d-%d of IDE controller %s\n",
               first_bar, last_bar, pci_name(dev));
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID,             PCI_ANY_ID,                     quirk_ide_bases );
+DECLARE_PCI_FIXUP_HEADER_ALL(quirk_ide_bases);
 
 /*
  *	Ensure C0 rev restreaming is off. This is normally done by
Index: linux-2.6/include/asm-generic/vmlinux.lds.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-generic/vmlinux.lds.h,v
retrieving revision 1.4
diff -u -p -r1.4 vmlinux.lds.h
--- linux-2.6/include/asm-generic/vmlinux.lds.h	13 Sep 2004 15:24:02 -0000	1.4
+++ linux-2.6/include/asm-generic/vmlinux.lds.h	22 Sep 2004 21:38:23 -0000
@@ -20,9 +20,11 @@
 	.pci_fixup        : AT(ADDR(.pci_fixup) - LOAD_OFFSET) {	\
 		VMLINUX_SYMBOL(__start_pci_fixups_header) = .;		\
 		*(.pci_fixup_header)					\
+		*(.pci_fixup_header_all)				\
 		VMLINUX_SYMBOL(__end_pci_fixups_header) = .;		\
 		VMLINUX_SYMBOL(__start_pci_fixups_final) = .;		\
 		*(.pci_fixup_final)					\
+		*(.pci_fixup_final_all)					\
 		VMLINUX_SYMBOL(__end_pci_fixups_final) = .;		\
 	}								\
 									\
Index: linux-2.6/include/linux/pci.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/linux/pci.h,v
retrieving revision 1.18
diff -u -p -r1.18 pci.h
--- linux-2.6/include/linux/pci.h	13 Sep 2004 15:24:12 -0000	1.18
+++ linux-2.6/include/linux/pci.h	22 Sep 2004 21:38:25 -0000
@@ -1011,15 +1011,22 @@ enum pci_fixup_pass {
 };
 
 /* Anonymous variables would be nice... */
-#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)					\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_header"))) = {				\
-		vendor, device, hook };
-
-#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)				\
-	static struct pci_fixup __pci_fixup_##vendor##device##hook __attribute_used__	\
-	__attribute__((__section__(".pci_fixup_final"))) = {				\
+#define DECLARE_PCI_FIXUP_SECTION(section, name, vendor, device, hook)	\
+	static struct pci_fixup __pci_fixup_##name			\
+	__attribute_used__ __attribute__((__section__( #section ))) = {	\
 		vendor, device, hook };
+#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook)			\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header,			\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_HEADER_ALL(hook)				\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header_all,		\
+			ALL_DEVICES##hook, PCI_ANY_ID, PCI_ANY_ID, hook)
+#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook)			\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final,			\
+			vendor##device##hook, vendor, device, hook)
+#define DECLARE_PCI_FIXUP_FINAL_ALL(hook)				\
+	DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final_all,			\
+			ALL_DEVICES##hook, PCI_ANY_ID, PCI_ANY_ID, hook)
 
 void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
 

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain