PCI, EDAC: fix ordering assign resource and bus_add
PATCH 1/1 PCI, EDAC: fix ordering assign resource and bus_add
We should assign unassigned resource before pci_bus_add_device.
as late one will enable driver and create sysfs file that will need pci io resources from assign unassigned code.
--- drivers/edac/i82875p_edac.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/edac/i82875p_edac.c +2 -1
@@ -293,13 +293,14 @@ static int i82875p_setup_overfl_dev(struif (dev ==NULL )return 1 ; +pci_bus_assign_resources (dev->bus); + err =pci_bus_add_device (dev);if (err) {i82875p_printk (KERN_ERR ,"%s(): pci_bus_add_device() Failed\n" , __func__); } -pci_bus_assign_resources (dev->bus); } *ovrfl_pdev = dev;
Quoted 6 lines · Yinghai Lu
We should assign unassigned resource before
pci_bus_add_device.as late one will enable driver and create sysfs file that will need pci io resources from assign unassigned code.
Signed-off-by Yinghai Lu <[email protected]>
Applied to pci/resource for v3.16, thanks!
Quoted 24 lines
--- drivers/edac/i82875p_edac.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Index:
linux-2.6/drivers/edac/i82875p_edac.c
===================================================================drivers/edac/i82875p_edac.c +2
-1@@ -293,13 +293,14 @@ static int i82875p_setup_overfl_dev(struif (dev ==NULL )return 1 ; +pci_bus_assign_resources (dev->bus); + err =pci_bus_add_device (dev);if (err) {i82875p_printk (KERN_ERR ,"%s(): pci_bus_add_device() Failed\n" , __func__); } -pci_bus_assign_resources (dev->bus); } *ovrfl_pdev = dev;
Quoted 9 lines · Bjorn Helgaas
Quoted 6 lines · Yinghai Lu
We should assign unassigned resource before
pci_bus_add_device.as late one will enable driver and create sysfs file that will need pci io resources from assign unassigned code.
Signed-off-by Yinghai Lu <[email protected]>Applied to pci/resource for v3.16, thanks!
This is touching drivers/edac/ and I guess I'm fine with it,
Quoted 12 lines
Quoted 12 lines
--- drivers/edac/i82875p_edac.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Index:
linux-2.6/drivers/edac/i82875p_edac.c
===================================================================drivers/edac/i82875p_edac.c +0
-0@@ -293,13 +293,14 @@ static int i82875p_setup_overfl_dev(struif (dev ==NULL )return 1 ;
Quoted 9 lines
drivers/edac/i82875p_edac.c static int i82875p_setup_overfl_dev(stru
Quoted 9 lines
drivers/edac/i82875p_edac.c +2
-1+pci_bus_assign_resources (dev->bus); + err =pci_bus_add_device (dev);if (err) {i82875p_printk (KERN_ERR ,"%s(): pci_bus_add_device() Failed\n" , __func__); } -pci_bus_assign_resources (dev->bus);a
... one question though: how are people using those pci_bus* functions supposed to know that pci_bus_assign_resources needs to go before pci_bus_add_device? The comment in pci_bus_add_device() mentions something about it but wouldn't it be possible to call pci_bus_assign_resources() in pci_bus_add_device() when resources are not assigned?
Sorry if my question is completely dumb - I have no presumption of knowing pci and haven't looked either. Just asking with my user hat on.
Thanks.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Bjorn Helgaas
maintainerQuoted 7 lines · Borislav Petkov
Quoted 6 lines · Bjorn Helgaas
Quoted 5 lines · Yinghai Lu
We should assign unassigned resource before
pci_bus_add_device.as late one will enable driver and create sysfs file that will need pci io resources from assign unassigned code.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Yinghai Lu <[email protected]>Applied to pci/resource for v3.16, thanks!
This is touching
drivers/edac/and I guess I'm fine with it,Acked-by Borislav Petkov <[email protected]>
Thanks, I should have waited for you. I was assuming the merge window would open on Monday, and I hoped for a day or two in -next, but now it sounds like we might have an -rc8, so I needn't have been in such a hurry.
Quoted 23 lines
Quoted 12 lines
Quoted 12 lines
--- drivers/edac/i82875p_edac.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Index:
linux-2.6/drivers/edac/i82875p_edac.c
===================================================================drivers/edac/i82875p_edac.c +0
-0@@ -293,13 +293,14 @@ static int i82875p_setup_overfl_dev(struif (dev ==NULL )return 1 ;Quoted 9 lines
drivers/edac/i82875p_edac.c static int i82875p_setup_overfl_dev(stru
Quoted 9 lines
drivers/edac/i82875p_edac.c +2
-1+pci_bus_assign_resources (dev->bus); + err =pci_bus_add_device (dev);if (err) {i82875p_printk (KERN_ERR ,"%s(): pci_bus_add_device() Failed\n" , __func__); } -pci_bus_assign_resources (dev->bus);a
Quoted 6 lines
... one question though: how are people using those pci_bus* functions supposed to know that
pci_bus_assign_resourcesneeds to go beforepci_bus_add_device? The comment inpci_bus_add_device()mentions something about it but wouldn't it be possible to callpci_bus_assign_resources()inpci_bus_add_device()when resources are not assigned?
Yeah, this is really a mess. The pci_bus_*() functions (at least pci_bus_add_device() and pci_bus_assign_resources()) are not really intended to be called by drivers at all -- they should only be used by the arch code that drives PCI enumeration. Eventually I'd like to even move them out of the arch code and into the PCI core.
In this i82875p case, it looks like this is basically a quirk that unhides a device. I think it's sort of dubious to poke at things the BIOS has explicitly hidden from the OS, but apart from that, if we turned this into an actual DECLARE_PCI_FIXUP_CLASS_EARLY() quirk on the 82875_0 device, I think the normal PCI enumeration process would find the 82875_6 device, and we could get rid of the whole "if (dev == NULL)" chunk.
Bjorn
Borislav Petkov
maintainerQuoted 4 lines · Bjorn Helgaas
Thanks, I should have waited for you. I was assuming the merge window would open on Monday, and I hoped for a day or two in -next, but now it sounds like we might have an -rc8, so I needn't have been in such a hurry.
Bah, no worries. :-) The patch is simple enough and is requiring your expertise so I wouldnt've said anything, except I wanted to ask about that pci_bus* functions ordering situation, so... :-)
Quoted 5 lines
Yeah, this is really a mess. The pci_bus_*() functions (at least
pci_bus_add_device()andpci_bus_assign_resources()) are not really intended to be called by drivers at all -- they should only be used by the arch code that drives PCI enumeration. Eventually I'd like to even move them out of the arch code and into the PCI core.
Ah, ok. pci_bus_add_device is called also in some x86 platform drivers, except the pci core itself, so yeah, it looks like you're trying to hide it. :)
Quoted 7 lines
In this i82875p case, it looks like this is basically a quirk that unhides a device. I think it's sort of dubious to poke at things the BIOS has explicitly hidden from the OS, but apart from that, if we turned this into an actual
DECLARE_PCI_FIXUP_CLASS_EARLY()quirk on the 82875_0 device, I think the normal PCI enumeration process would find the 82875_6 device, and we could get rid of the whole "if (dev == NULL)" chunk.
Yeah, I don't think that's worth it - this driver is probably ancient now, if I trust this:
http://ark.intel.com/products/27711/Intel-82875P-Memory-Controller
Launch Date: Q1'02 - that's a century ago in HW years. :-P
I guess we can leave it at that - even if we did a quirk, I'm at a loss as to where or who could test it. :-)
Thanks.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--