Message ID | 1367969371-27414-1-git-send-email-yinghai@kernel.org |
---|---|
State | Not Applicable |
Headers | show |
On Tue, May 07, 2013 at 04:29:31PM -0700, Yinghai Lu wrote: > 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 <yinghai@kernel.org> Applied to pci/resource for v3.16, thanks! > --- > drivers/edac/i82875p_edac.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > Index: linux-2.6/drivers/edac/i82875p_edac.c > =================================================================== > --- linux-2.6.orig/drivers/edac/i82875p_edac.c > +++ linux-2.6/drivers/edac/i82875p_edac.c > @@ -293,13 +293,14 @@ static int i82875p_setup_overfl_dev(stru > if (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; -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 30, 2014 at 11:01:03AM -0600, Bjorn Helgaas wrote: > On Tue, May 07, 2013 at 04:29:31PM -0700, Yinghai Lu wrote: > > 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 <yinghai@kernel.org> > > 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 <bp@suse.de> > > --- > > drivers/edac/i82875p_edac.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > Index: linux-2.6/drivers/edac/i82875p_edac.c > > =================================================================== > > --- linux-2.6.orig/drivers/edac/i82875p_edac.c > > +++ linux-2.6/drivers/edac/i82875p_edac.c > > @@ -293,13 +293,14 @@ static int i82875p_setup_overfl_dev(stru > > if (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);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.
On Fri, May 30, 2014 at 3:46 PM, Borislav Petkov <bp@alien8.de> wrote: > On Fri, May 30, 2014 at 11:01:03AM -0600, Bjorn Helgaas wrote: >> On Tue, May 07, 2013 at 04:29:31PM -0700, Yinghai Lu wrote: >> > 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 <yinghai@kernel.org> >> >> 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 <bp@suse.de> 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. >> > --- >> > drivers/edac/i82875p_edac.c | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > >> > Index: linux-2.6/drivers/edac/i82875p_edac.c >> > =================================================================== >> > --- linux-2.6.orig/drivers/edac/i82875p_edac.c >> > +++ linux-2.6/drivers/edac/i82875p_edac.c >> > @@ -293,13 +293,14 @@ static int i82875p_setup_overfl_dev(stru >> > if (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);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? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 30, 2014 at 04:00:17PM -0600, Bjorn Helgaas wrote: > 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... :-) > 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. 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. :) > 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.
Index: linux-2.6/drivers/edac/i82875p_edac.c =================================================================== --- linux-2.6.orig/drivers/edac/i82875p_edac.c +++ linux-2.6/drivers/edac/i82875p_edac.c @@ -293,13 +293,14 @@ static int i82875p_setup_overfl_dev(stru if (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;
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 <yinghai@kernel.org> --- drivers/edac/i82875p_edac.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html