PCI: quirk related clean up
Subsystems (4)
PATCH 1/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
Thanks
Yinghai
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.
--- drivers/pci/quirks.c | 19 ++++++++++-------- include/linux/pci.h | 53 ++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 19 deletions(-)
drivers/pci/quirks.c +11 -8
@@ -2914,14 +2914,15 @@ static void do_one_fixup_debug(void (*fnktime_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)); }/*
@@ -2961,17 +2962,19 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INstatic 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
PATCH 2/4 PCI: Specify class for quirk entry with PCI_ANY_ID
In reply to Yinghai LuUse 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
--- 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
@@ -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
@@ -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(ststruct 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(stdev_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
@@ -901,4 +901,5 @@ static void __devinit quirk_usb_early_haelse 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);
PATCH 3/4 PCI: Move out pci reassigndev resource alignment out of quirk
In reply to Yinghai LuThat looks like abusing quirk framework.
calling it directly in pci_add_device. or do we have good place for it?
--- 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
@@ -3694,6 +3694,68 @@ int pci_is_reassigndev(struct pci_dev *dreturn (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
@@ -230,11 +230,8 @@ static inline int pci_ari_enabled(structreturn 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
@@ -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 pcpci_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
@@ -1426,6 +1426,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;
PATCH 4/4 PCI: quirk print dev name with duration
In reply to Yinghai LuSo could find out which device is involved with the calling
--- drivers/pci/quirks.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/pci/quirks.c +4 -3
@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fnktime_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)); }/*
Re: [PATCH 4/4] PCI: quirk print dev name with duration
Quoted 1 line · Yinghai Lu
So could find out which device is involved with the calling
[]
drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858
Quoted 8 lines
+++
linux-2.6/drivers/pci/quirks.cdrivers/pci/quirks.c +2
-1@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fnktime_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));
Might be better to use dev_dbg
dev_dbg(&dev->dev, "calling %pF @ %i\n", fn, task_pid_nr(current));
[]
drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2865
Quoted 4 lines
drivers/pci/quirks.c +2
-2-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));
dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n", fn, duration);
Re: [PATCH 4/4] PCI: quirk print dev name with duration
Quoted 18 lines · Joe Perches
Quoted 1 line · Yinghai Lu
So could find out which device is involved with the calling
[]
drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858
Quoted 8 lines
+++
linux-2.6/drivers/pci/quirks.cdrivers/pci/quirks.c +2
-1@@ -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));Might be better to use
dev_dbgdev_dbg(&dev->dev, "calling %pF @ %i\n", fn,
task_pid_nr(current));[]
Quoted 7 lines
drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2865
Quoted 4 lines
drivers/pci/quirks.c +2
-2- 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));dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n", fn, duration);
Not sure if that is going to break Arjan 's tool that analyze the boot time.
Thanks
Yinghai
Re: [PATCH 4/4] PCI: quirk print dev name with duration
Quoted 24 lines · Yinghai Lu
Quoted 23 lines · Joe Perches
Quoted 1 line · Yinghai Lu
So could find out which device is involved with the calling
[]
drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858
Quoted 8 lines
+++
linux-2.6/drivers/pci/quirks.cdrivers/pci/quirks.c +2
-1@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fnktime_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));Might be better to use
dev_dbgdev_dbg(&dev->dev, "calling %pF @ %i\n", fn, task_pid_nr(current));[]
Quoted 4 lines
drivers/pci/quirks.c +2
-2-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));
Quoted 4 lines
Quoted 2 lines
dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n", fn, duration);Not sure if that is going to break Arjan 's tool that analyze the boot time.
That depends on the tool code.
Either change might break it.
Did you test your change with Arjan's tool?
Do you have a link to the tool?
Re: [PATCH 4/4] PCI: quirk print dev name with duration
Quoted 25 lines · Joe Perches
Quoted 24 lines · Yinghai Lu
Quoted 23 lines · Joe Perches
Quoted 1 line · Yinghai Lu
So could find out which device is involved with the calling
[]
drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858
Quoted 8 lines
+++
linux-2.6/drivers/pci/quirks.cdrivers/pci/quirks.c +2
-1@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fnktime_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));Might be better to use
dev_dbgdev_dbg(&dev->dev, "calling %pF @ %i\n", fn, task_pid_nr(current));[]
Quoted 4 lines
drivers/pci/quirks.c +2
-2-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));
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n", fn, duration);Not sure if that is going to break Arjan 's tool that analyze the boot time.
That depends on the tool code.
[]
Quoted 1 line
Do you have a link to the tool?
That tool would be scripts/bootgraph.pl and usingdev_dbg would break it unless it's adapted to the
change. Your change seems fine for now.
Bjorn Helgaas
maintainerRe: [PATCH 4/4] PCI: quirk print dev name with duration
Quoted 28 lines · Joe Perches
Quoted 27 lines · Joe Perches
Quoted 26 lines · Yinghai Lu
Quoted 25 lines · Joe Perches
Quoted 1 line · Yinghai Lu
So could find out which device is involved with the calling
[]
drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858
Quoted 8 lines
+++
linux-2.6/drivers/pci/quirks.cdrivers/pci/quirks.c +2
-1@@ -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));Might be better to use
dev_dbgdev_dbg(&dev->dev, "calling %pF @ %i\n", fn,
task_pid_nr(current));[]
drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2865
Quoted 4 lines
drivers/pci/quirks.c +2
-2- 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));dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n", fn, duration);
Quoted 8 lines
Quoted 2 lines
Quoted 1 line
Not sure if that is going to break Arjan 's tool that analyze the boot time.
That depends on the tool code.
[]
Quoted 1 line
Do you have a link to the tool?
That tool would be
scripts/bootgraph.pland usingdev_dbgwould break it unless it's adapted to the
change. Your change seems fine for now.
Since we're adding new functionality (analyzing quirk durations), I think it would be fair game to update the tool at the same time so it could parse the dev_dbg() format.
For things related specifically to a device, we have a pretty good record of trying to use dev_*() consistently, so I do have a preference for continuing that.
If you do change, remember that dev_dbg() is different from printk(KERN_DEBUG) in that printk(KERN_DEBUG) is always compiled-in and always goes to the dmesg, but dev_dbg() is only compiled in when DEBUG is defined, so dev_printk(KERN_DEBUG) would be a more direct replacement for printk(KERN_DEBUG).
Bjorn
Re: [PATCH 4/4] PCI: quirk print dev name with duration
Quoted 5 lines · Bjorn Helgaas
If you do change, remember that
dev_dbg()is different fromprintk(KERN_DEBUG)in thatprintk(KERN_DEBUG)is always compiled-in and always goes to the dmesg, butdev_dbg()is only compiled in when DEBUG is defined, sodev_printk(KERN_DEBUG)would be a more direct replacement forprintk(KERN_DEBUG).
Correct.
Which is why I usually add #define DEBUG or an equivalent CONFIG option before those are converted. Doing so allows control via dynamic debug.