2012-02-16 05:41:09

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 0/4] 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


2012-02-16 05:41:53

by Yinghai Lu

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

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(-)

Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ 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)
Index: linux-2.6/drivers/pci/pci.h
===================================================================
--- linux-2.6.orig/drivers/pci/pci.h
+++ 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 {
Index: linux-2.6/drivers/pci/quirks.c
===================================================================
--- linux-2.6.orig/drivers/pci/quirks.c
+++ linux-2.6/drivers/pci/quirks.c
@@ -32,69 +32,6 @@
#include "pci.h"

/*
- * This quirk 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.
- */
-static void __devinit quirk_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);
- }
-}
-DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, quirk_resource_alignment);
-
-/*
* Decoding should be disabled for a PCI device during BAR sizing to avoid
* conflict. But doing so may cause problems on host bridge and perhaps other
* key system devices. For devices that need to have mmio decoding always-on,
Index: linux-2.6/drivers/pci/setup-res.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-res.c
+++ 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)
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ 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;

2012-02-16 05:41:48

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH -v2 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(-)

Index: linux-2.6/drivers/pci/quirks.c
===================================================================
--- linux-2.6.orig/drivers/pci/quirks.c
+++ 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[];
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -1377,7 +1377,10 @@ static inline void pci_resource_to_user(
*/

struct pci_fixup {
- u16 vendor, device; /* You can use PCI_ANY_ID here of course */
+ u16 vendor; /* You can use PCI_ANY_ID here of course */
+ u16 device; /* You can use PCI_ANY_ID here of course */
+ u32 class; /* You can use PCI_ANY_ID here too */
+ unsigned int cls_shift; /* should be 0, 8, 16 */
void (*hook)(struct pci_dev *dev);
};

@@ -1392,30 +1395,58 @@ enum pci_fixup_pass {
};

/* Anonymous variables would be nice... */
-#define DECLARE_PCI_FIXUP_SECTION(section, name, vendor, device, hook) \
- static const struct pci_fixup __pci_fixup_##name __used \
- __attribute__((__section__(#section))) = { vendor, device, hook };
+#define DECLARE_PCI_FIXUP_SECTION(section, name, vendor, device, c, cs, hook)\
+ static const struct pci_fixup const __pci_fixup_##name __used \
+ __attribute__((__section__(#section), aligned((sizeof(void *))))) \
+ = { vendor, device, c, cs, hook };
+
+#define DECLARE_PCI_FIXUP_CLASS_EARLY(vendor, device, cls, clt, hook) \
+ DECLARE_PCI_FIXUP_SECTION(.pci_fixup_early, \
+ vendor##device##hook, vendor, device, cls, clt, hook)
+#define DECLARE_PCI_FIXUP_CLASS_HEADER(vendor, device, cls, clt, hook) \
+ DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header, \
+ vendor##device##hook, vendor, device, cls, clt, hook)
+#define DECLARE_PCI_FIXUP_CLASS_FINAL(vendor, device, cls, clt, hook) \
+ DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final, \
+ vendor##device##hook, vendor, device, cls, clt, hook)
+#define DECLARE_PCI_FIXUP_CLASS_ENABLE(vendor, device, cls, clt, hook) \
+ DECLARE_PCI_FIXUP_SECTION(.pci_fixup_enable, \
+ vendor##device##hook, vendor, device, cls, clt, hook)
+#define DECLARE_PCI_FIXUP_CLASS_RESUME(vendor, device, cls, clt, hook) \
+ DECLARE_PCI_FIXUP_SECTION(.pci_fixup_resume, \
+ resume##vendor##device##hook, vendor, device, cls, clt, hook)
+#define DECLARE_PCI_FIXUP_CLASS_RESUME_EARLY(vendor, device, cls, clt, hook) \
+ DECLARE_PCI_FIXUP_SECTION(.pci_fixup_resume_early, \
+ resume_early##vendor##device##hook, vendor, device, \
+ cls, clt, hook)
+#define DECLARE_PCI_FIXUP_CLASS_SUSPEND(vendor, device, cls, clt, hook) \
+ DECLARE_PCI_FIXUP_SECTION(.pci_fixup_suspend, \
+ suspend##vendor##device##hook, vendor, device, cls, clt, hook)
+
#define DECLARE_PCI_FIXUP_EARLY(vendor, device, hook) \
DECLARE_PCI_FIXUP_SECTION(.pci_fixup_early, \
- vendor##device##hook, vendor, device, hook)
+ vendor##device##hook, vendor, device, PCI_ANY_ID, 0, hook)
#define DECLARE_PCI_FIXUP_HEADER(vendor, device, hook) \
DECLARE_PCI_FIXUP_SECTION(.pci_fixup_header, \
- vendor##device##hook, vendor, device, hook)
+ vendor##device##hook, vendor, device, PCI_ANY_ID, 0, hook)
#define DECLARE_PCI_FIXUP_FINAL(vendor, device, hook) \
DECLARE_PCI_FIXUP_SECTION(.pci_fixup_final, \
- vendor##device##hook, vendor, device, hook)
+ vendor##device##hook, vendor, device, PCI_ANY_ID, 0, hook)
#define DECLARE_PCI_FIXUP_ENABLE(vendor, device, hook) \
DECLARE_PCI_FIXUP_SECTION(.pci_fixup_enable, \
- vendor##device##hook, vendor, device, hook)
+ vendor##device##hook, vendor, device, PCI_ANY_ID, 0, hook)
#define DECLARE_PCI_FIXUP_RESUME(vendor, device, hook) \
DECLARE_PCI_FIXUP_SECTION(.pci_fixup_resume, \
- resume##vendor##device##hook, vendor, device, hook)
+ resume##vendor##device##hook, vendor, device, \
+ PCI_ANY_ID, 0, hook)
#define DECLARE_PCI_FIXUP_RESUME_EARLY(vendor, device, hook) \
DECLARE_PCI_FIXUP_SECTION(.pci_fixup_resume_early, \
- resume_early##vendor##device##hook, vendor, device, hook)
+ resume_early##vendor##device##hook, vendor, device, \
+ PCI_ANY_ID, 0, hook)
#define DECLARE_PCI_FIXUP_SUSPEND(vendor, device, hook) \
DECLARE_PCI_FIXUP_SECTION(.pci_fixup_suspend, \
- suspend##vendor##device##hook, vendor, device, hook)
+ suspend##vendor##device##hook, vendor, device, \
+ PCI_ANY_ID, 0, hook)

#ifdef CONFIG_PCI_QUIRKS
void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);

2012-02-16 05:41:59

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 4/4] PCI: quirk print dev name with duration

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(-)

Index: linux-2.6/drivers/pci/quirks.c
===================================================================
--- linux-2.6.orig/drivers/pci/quirks.c
+++ 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));
}

/*

2012-02-16 05:41:51

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH 2/4] PCI: Specify class for quirk entry with PCI_ANY_ID

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(-)

Index: linux-2.6/arch/x86/kernel/pci-dma.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/pci-dma.c
+++ 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
Index: linux-2.6/arch/x86/pci/fixup.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/fixup.c
+++ 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[] = {
Index: linux-2.6/drivers/pci/quirks.c
===================================================================
--- linux-2.6.orig/drivers/pci/quirks.c
+++ linux-2.6/drivers/pci/quirks.c
@@ -102,10 +102,10 @@ DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI
*/
static void __devinit quirk_mmio_always_on(struct pci_dev *dev)
{
- if ((dev->class >> 8) == PCI_CLASS_BRIDGE_HOST)
- dev->mmio_always_on = 1;
+ dev->mmio_always_on = 1;
}
-DECLARE_PCI_FIXUP_EARLY(PCI_ANY_ID, PCI_ANY_ID, quirk_mmio_always_on);
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID,
+ PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on);

/* The Mellanox Tavor device gives false positive parity errors
* Mark this device with a broken_parity_status, to allow
@@ -1004,12 +1004,12 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
*/
static void quirk_cardbus_legacy(struct pci_dev *dev)
{
- if ((PCI_CLASS_BRIDGE_CARDBUS << 8) ^ dev->class)
- 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_RESUME_EARLY(PCI_ANY_ID, PCI_ANY_ID, quirk_cardbus_legacy);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+ PCI_CLASS_BRIDGE_CARDBUS, 8, quirk_cardbus_legacy);
+DECLARE_PCI_FIXUP_CLASS_RESUME_EARLY(PCI_ANY_ID, PCI_ANY_ID,
+ PCI_CLASS_BRIDGE_CARDBUS, 8, quirk_cardbus_legacy);

/*
* Following the PCI ordering rules is optional on the AMD762. I'm not
@@ -1166,17 +1166,20 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_IN

static void __devinit quirk_no_ata_d3(struct pci_dev *pdev)
{
- /* Quirk the legacy ATA devices only. The AHCI ones are ok */
- if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE)
- pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
+ pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
}
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, PCI_ANY_ID, quirk_no_ata_d3);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, PCI_ANY_ID, quirk_no_ata_d3);
+/* Quirk the legacy ATA devices only. The AHCI ones are ok */
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_SERVERWORKS, PCI_ANY_ID,
+ PCI_CLASS_STORAGE_IDE, 8, quirk_no_ata_d3);
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_ATI, PCI_ANY_ID,
+ PCI_CLASS_STORAGE_IDE, 8, quirk_no_ata_d3);
/* ALi loses some register settings that we cannot then restore */
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, quirk_no_ata_d3);
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID,
+ PCI_CLASS_STORAGE_IDE, 8, quirk_no_ata_d3);
/* VIA comes back fine but we need to keep it alive or ACPI GTM failures
occur when mode detecting */
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_VIA, PCI_ANY_ID, quirk_no_ata_d3);
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_VIA, PCI_ANY_ID,
+ PCI_CLASS_STORAGE_IDE, 8, quirk_no_ata_d3);

/* This was originally an Alpha specific thing, but it really fits here.
* The i82375 PCI/EISA bridge appears as non-classified. Fix that.
@@ -1954,7 +1957,8 @@ static void __devinit quirk_e100_interru

iounmap(csr);
}
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_e100_interrupt);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
+ PCI_CLASS_NETWORK_ETHERNET, 8, quirk_e100_interrupt);

/*
* The 82575 and 82598 may experience data corruption issues when transitioning
@@ -2818,12 +2822,11 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_IN
static void __devinit fixup_ti816x_class(struct pci_dev* dev)
{
/* TI 816x devices do not have class code set when in PCIe boot mode */
- if (dev->class == PCI_CLASS_NOT_DEFINED) {
- dev_info(&dev->dev, "Setting PCI class for 816x PCIe device\n");
- dev->class = PCI_CLASS_MULTIMEDIA_VIDEO;
- }
+ dev_info(&dev->dev, "Setting PCI class for 816x PCIe device\n");
+ dev->class = PCI_CLASS_MULTIMEDIA_VIDEO;
}
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_TI, 0xb800, fixup_ti816x_class);
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_TI, 0xb800,
+ PCI_CLASS_NOT_DEFINED, 0, fixup_ti816x_class);

/* Some PCIe devices do not work reliably with the claimed maximum
* payload size supported.
Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================
--- linux-2.6.orig/drivers/usb/host/pci-quirks.c
+++ 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);

2012-02-23 20:44:34

by Jesse Barnes

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

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.
>
> 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,
--
Jesse Barnes, Intel Open Source Technology Center


Attachments:
signature.asc (836.00 B)

2012-02-24 06:40:17

by Yinghai Lu

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

On Thu, Feb 23, 2012 at 12:44 PM, Jesse Barnes <[email protected]> wrote:
>
> I like this series, a few comments:
> ?- class and class_shift everywhere (in pci_fixup {} and the new
> ? ?macros) for readability

ok.

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

yes

will send updated patches shortly.

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

yes, you can apply that separately.

Thanks

Yinghai
yes.

2012-02-24 22:37:48

by Jesse Barnes

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

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.

--
Jesse Barnes, Intel Open Source Technology Center


Attachments:
signature.asc (836.00 B)

2012-02-24 22:39:27

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH 4/4] PCI: quirk print dev name with duration

On Wed, 15 Feb 2012 21:40:32 -0800
Yinghai Lu <[email protected]> wrote:

> 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]>

Applied, thanks.

--
Jesse Barnes, Intel Open Source Technology Center


Attachments:
signature.asc (836.00 B)