Skip to contentSkip to conversation

PCI: quirk related clean up

PATCH Applied Yinghai Lu 2 participants 9 messages Feb 16, 2012 – Feb 24, 2012

Subsystems (4)
x86 USB PCI USB xHCI driver
lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

cover PCI: quirk related clean up

After commit:

| commit 3209874a1da2c51c7325e601d9634189ee178ad6
| Author: Arjan van de Ven <[email protected]>
| Date:   Mon Jan 30 20:52:07 2012 -0800
|
|    PCI: Annotate PCI quirks in initcall_debug style

will have lots of print out for quirks when initcall_debug is specified.

It turns out most of them are not really called for the devices because quirks itself will check class id and bail out early.

Try to class into quirk declaration. So we could avoid dip into these quirks.

[PATCH 1/4] PCI: Add class support in quirk handling
[PATCH 2/4] PCI: Specify class for quirk entry with PCI_ANY_ID [PATCH 3/4] PCI: Move out pci reassigndev resource alignment out of quirk [PATCH 4/4] PCI: quirk print dev name with duration

Resending

Thanks

Yinghai

author
#1 lore raw

PATCH 1/4 PCI: Add class support in quirk handling

Recently there is new support to make quirk calling will report duration time. That will boot log get more print out with initcall_debug is specified.

Found a lot of quirks are calling for not related devices.

Reason is quirk frame do not support class handling. So quirk code have to use PCI_ANY_ID for vendor/device and let quirk code itself to check class inside the func.

The patch add class and cls_shift into struct pci_fixup. Also update related macro to accept the class and shift.

-v2: fix v1 that left over of sparated patch.

Signed-off-by Yinghai Lu <[email protected]>
---
 drivers/pci/quirks.c |   12 ++++++-----
 include/linux/pci.h  |   53 ++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 49 insertions(+), 16 deletions(-)
drivers/pci/quirks.c +7 -5
Index: linux-2.6/drivers/pci/quirks.c
===================================================================
@@ -2961,17 +2961,19 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_IN
 static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
 			  struct pci_fixup *end)
 {
-	while (f < end) {
-		if ((f->vendor == dev->vendor || f->vendor == (u16) PCI_ANY_ID) &&
-		    (f->device == dev->device || f->device == (u16) PCI_ANY_ID)) {
+	for (; f < end; f++)
+		if ((f->class == (u32) (dev->class >> f->cls_shift) ||
+		     f->class == (u32) PCI_ANY_ID) &&
+		    (f->vendor == dev->vendor ||
+		     f->vendor == (u16) PCI_ANY_ID) &&
+		    (f->device == dev->device ||
+		     f->device == (u16) PCI_ANY_ID)) {
 			dev_dbg(&dev->dev, "calling %pF\n", f->hook);
 			if (initcall_debug)
 				do_one_fixup_debug(f->hook, dev);
 			else
 				f->hook(dev);
 		}
-		f++;
-	}
 }
 
 extern struct pci_fixup __start_pci_fixups_early[];
include/linux/pci.h +42 -11

79 lines not on this page · raw message

#2 lore raw

Re: [PATCH -v2 1/4] PCI: Add class support in quirk handling

Quoted 5 lines · On Wed, 15 Feb 2012 21:40:29 -0800 Yinghai Lu <[email protected]> wrote:

Recently there is new support to make quirk calling will report duration time. That will boot log get more print out with initcall_debug is specified.

Found a lot of quirks are calling for not related devices.

Quoted 8 lines

Reason is quirk frame do not support class handling. So quirk code have to use PCI_ANY_ID for vendor/device and let quirk code itself to check class inside the func.

The patch add class and cls_shift into struct pci_fixup. Also update related macro to accept the class and shift.

-v2: fix v1 that left over of sparated patch.

I like this series, a few comments:
- class and class_shift everywhere (in pci_fixup {} and the new macros) for readability
- split the individual quirk fixes into separate patches (makes it easier to revert or identify if one breaks)
- the reassigndev patch looks like a good one independent of the series

Thanks,

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
author
#3 lore raw

Re: [PATCH -v2 1/4] PCI: Add class support in quirk handling

Quoted 4 lines · Jesse Barnes

I like this series, a few comments:
 - class and class_shift everywhere (in pci_fixup {} and the new macros) for readability

ok.

Quoted 2 lines

 - split the individual quirk fixes into separate patches (makes it easier to revert or identify if one breaks)

yes

will send updated patches shortly.

Quoted 2 lines

 - the reassigndev patch looks like a good one independent of the series

yes, you can apply that separately.

Thanks

Yinghai
yes.

author
#4 lore raw

PATCH 2/4 PCI: Specify class for quirk entry with PCI_ANY_ID

In reply to Yinghai Lu

Use quirk framework class checking instead check class inside every quirk func.

That reduce quirk calling significent esp on Intel 8 sockets systems that have lots of device.
- quirk_e100_interrupt
- quirk_usb_early_handoff
- pci_fixup_video
- quirk_mmio_always_on
- quirk_cardbus_legacy

Signed-off-by Yinghai Lu <[email protected]>
---
 arch/x86/kernel/pci-dma.c     |    5 ++--
 arch/x86/pci/fixup.c          |   12 ++++-------
 drivers/pci/quirks.c          |   43 ++++++++++++++++++++++--------------------
 drivers/usb/host/pci-quirks.c |    3 +-
 4 files changed, 33 insertions(+), 30 deletions(-)
arch/x86/kernel/pci-dma.c +3 -2
Index: linux-2.6/arch/x86/kernel/pci-dma.c
===================================================================
@@ -262,10 +262,11 @@ rootfs_initcall(pci_iommu_init);
 
 static __devinit void via_no_dac(struct pci_dev *dev)
 {
-	if ((dev->class >> 8) == PCI_CLASS_BRIDGE_PCI && forbid_dac == 0) {
+	if (forbid_dac == 0) {
 		dev_info(&dev->dev, "disabling DAC on VIA PCI bridge\n");
 		forbid_dac = 1;
 	}
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_VIA, PCI_ANY_ID, via_no_dac);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_VIA, PCI_ANY_ID,
+				PCI_CLASS_BRIDGE_PCI, 8, via_no_dac);
 #endif
arch/x86/pci/fixup.c +5 -7
Index: linux-2.6/arch/x86/pci/fixup.c
===================================================================
@@ -164,11 +164,11 @@ DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_V
  */
 static void __devinit pci_fixup_transparent_bridge(struct pci_dev *dev)
 {
-	if ((dev->class >> 8) == PCI_CLASS_BRIDGE_PCI &&
-	    (dev->device & 0xff00) == 0x2400)
+	if ((dev->device & 0xff00) == 0x2400)
 		dev->transparent = 1;
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_fixup_transparent_bridge);
+DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
+			 PCI_CLASS_BRIDGE_PCI, 8, pci_fixup_transparent_bridge);
 
 /*
  * Fixup for C1 Halt Disconnect problem on nForce2 systems.

@@ -322,9 +322,6 @@ static void __devinit pci_fixup_video(st
 	struct pci_bus *bus;
 	u16 config;
 
-	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
-		return;
-
 	/* Is VGA routed to us? */
 	bus = pdev->bus;
 	while (bus) {

@@ -353,7 +350,8 @@ static void __devinit pci_fixup_video(st
 		dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
 	}
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+				PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
 
 
 static const struct dmi_system_id __devinitconst msi_k8t_dmi_table[] = {
drivers/pci/quirks.c +23 -20

81 lines not on this page · raw message

drivers/usb/host/pci-quirks.c +2 -1
Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================
@@ -882,4 +882,5 @@ static void __devinit quirk_usb_early_ha
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI)
 		quirk_usb_handoff_xhci(pdev);
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_usb_early_handoff);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+			PCI_CLASS_SERIAL_USB, 8, quirk_usb_early_handoff);
author
#5 lore raw

PATCH 3/4 PCI: Move out pci reassigndev resource alignment out of quirk

In reply to Yinghai Lu

That looks like abusing quirk framework.

calling it directly in pci_add_device. or do we have good place for it?

Signed-off-by Yinghai Lu <[email protected]>
---
 drivers/pci/pci.c       |   62 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h       |    5 ---
 drivers/pci/probe.c     |    3 ++
 drivers/pci/quirks.c    |   63 ------------------------------------------------
 drivers/pci/setup-res.c |    4 ---
 5 files changed, 66 insertions(+), 71 deletions(-)
drivers/pci/pci.c +62 -0
Index: linux-2.6/drivers/pci/pci.c
===================================================================
@@ -3694,6 +3694,68 @@ int pci_is_reassigndev(struct pci_dev *d
 	return (pci_specified_resource_alignment(dev) != 0);
 }
 
+/*
+ * This function disables memory decoding and releases memory resources
+ * of the device specified by kernel's boot parameter 'pci=resource_alignment='.
+ * It also rounds up size to specified alignment.
+ * Later on, the kernel will assign page-aligned memory resource back
+ * to the device.
+ */
+void pci_reassigndev_resource_alignment(struct pci_dev *dev)
+{
+	int i;
+	struct resource *r;
+	resource_size_t align, size;
+	u16 command;
+
+	if (!pci_is_reassigndev(dev))
+		return;
+
+	if (dev->hdr_type == PCI_HEADER_TYPE_NORMAL &&
+	    (dev->class >> 8) == PCI_CLASS_BRIDGE_HOST) {
+		dev_warn(&dev->dev,
+			"Can't reassign resources to host bridge.\n");
+		return;
+	}
+
+	dev_info(&dev->dev,
+		"Disabling memory decoding and releasing memory resources.\n");
+	pci_read_config_word(dev, PCI_COMMAND, &command);
+	command &= ~PCI_COMMAND_MEMORY;
+	pci_write_config_word(dev, PCI_COMMAND, command);
+
+	align = pci_specified_resource_alignment(dev);
+	for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
+		r = &dev->resource[i];
+		if (!(r->flags & IORESOURCE_MEM))
+			continue;
+		size = resource_size(r);
+		if (size < align) {
+			size = align;
+			dev_info(&dev->dev,
+				"Rounding up size of resource #%d to %#llx.\n",
+				i, (unsigned long long)size);
+		}
+		r->end = size - 1;
+		r->start = 0;
+	}
+	/* Need to disable bridge's resource window,
+	 * to enable the kernel to reassign new resource
+	 * window later on.
+	 */
+	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
+	    (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
+		for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
+			r = &dev->resource[i];
+			if (!(r->flags & IORESOURCE_MEM))
+				continue;
+			r->end = resource_size(r) - 1;
+			r->start = 0;
+		}
+		pci_disable_bridge_window(dev);
+	}
+}
+
 ssize_t pci_set_resource_alignment_param(const char *buf, size_t count)
 {
 	if (count > RESOURCE_ALIGNMENT_PARAM_SIZE - 1)
drivers/pci/pci.h +1 -4
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -228,11 +228,8 @@ static inline int pci_ari_enabled(struct
 	return bus->self && bus->self->ari_enabled;
 }
 
-#ifdef CONFIG_PCI_QUIRKS
-extern int pci_is_reassigndev(struct pci_dev *dev);
-resource_size_t pci_specified_resource_alignment(struct pci_dev *dev);
+void pci_reassigndev_resource_alignment(struct pci_dev *dev);
 extern void pci_disable_bridge_window(struct pci_dev *dev);
-#endif
 
 /* Single Root I/O Virtualization */
 struct pci_sriov {
drivers/pci/quirks.c +0 -63

69 lines not on this page · raw message

drivers/pci/setup-res.c +0 -4
Index: linux-2.6/drivers/pci/setup-res.c
===================================================================
@@ -114,7 +114,6 @@ int pci_claim_resource(struct pci_dev *d
 }
 EXPORT_SYMBOL(pci_claim_resource);
 
-#ifdef CONFIG_PCI_QUIRKS
 void pci_disable_bridge_window(struct pci_dev *dev)
 {
 	dev_info(&dev->dev, "disabling bridge mem windows\n");

@@ -127,9 +126,6 @@ void pci_disable_bridge_window(struct pc
 	pci_write_config_dword(dev, PCI_PREF_MEMORY_BASE, 0x0000fff0);
 	pci_write_config_dword(dev, PCI_PREF_BASE_UPPER32, 0xffffffff);
 }
-#endif	/* CONFIG_PCI_QUIRKS */
-
-
 
 static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
 		int resno, resource_size_t size, resource_size_t align)
drivers/pci/probe.c +3 -0
Index: linux-2.6/drivers/pci/probe.c
===================================================================
@@ -1227,6 +1227,9 @@ void pci_device_add(struct pci_dev *dev,
 	/* Fix up broken headers */
 	pci_fixup_device(pci_fixup_header, dev);
 
+	/* moved out from quirk header fixup code */
+	pci_reassigndev_resource_alignment(dev);
+
 	/* Clear the state_saved flag. */
 	dev->state_saved = false;
Applied
#6 lore raw

Re: [PATCH 3/4] PCI: Move out pci reassigndev resource alignment out of quirk

Quoted 5 lines · On Wed, 15 Feb 2012 21:40:31 -0800 Yinghai Lu <[email protected]> wrote:

That looks like abusing quirk framework.

calling it directly in pci_add_device. or do we have good place for it?

Signed-off-by Yinghai Lu <[email protected]>

Applied, thanks.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
author
#7 lore raw

PATCH 4/4 PCI: quirk print dev name with duration

In reply to Yinghai Lu

So could find out which device is involved with the calling

Signed-off-by Yinghai Lu <[email protected]>
Cc Arjan van de Ven <[email protected]>
---
 drivers/pci/quirks.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/pci/quirks.c +4 -3
Index: linux-2.6/drivers/pci/quirks.c
===================================================================
@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fn
 	ktime_t calltime, delta, rettime;
 	unsigned long long duration;
 
-	printk(KERN_DEBUG "calling  %pF @ %i\n", fn, task_pid_nr(current));
+	printk(KERN_DEBUG "calling  %pF @ %i for %s\n",
+			fn, task_pid_nr(current), dev_name(&dev->dev));
 	calltime = ktime_get();
 	fn(dev);
 	rettime = ktime_get();
 	delta = ktime_sub(rettime, calltime);
 	duration = (unsigned long long) ktime_to_ns(delta) >> 10;
-	printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs\n", fn,
-		duration);
+	printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs for %s\n",
+			fn, duration, dev_name(&dev->dev));
 }
 
 /*