| Message ID | 1430460188-31343-2-git-send-email-gwshan@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi Gavin, [Please run "git log --oneline drivers/pci/setup-bus.c" and observe the capitalization convention.] On Fri, May 01, 2015 at 04:02:48PM +1000, Gavin Shan wrote: > Currently, PowerPC PowerNV platform utilizes ppc_md.pcibios_fixup(), > which is called for once after PCI probing and resource assignment > are completed, to allocate platform required resources for PCI devices: > PE#, IO and MMIO mapping, DMA address translation (TCE) table etc. > Obviously, it's not hotplug friendly. > > The patch adds weak function pcibios_setup_bridge(), which is called > by pci_setup_bridge(). PowerPC PowerNV platform will reuse the function > to assign above platform required resources to newly added PCI devices, > in order to support PCI hotplug on PowerPC PowerNV platform. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > --- > drivers/pci/setup-bus.c | 12 +++++++++--- > include/linux/pci.h | 1 + > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 4fd0cac..a7d0c3c 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -674,7 +674,8 @@ static void pci_setup_bridge_mmio_pref(struct pci_dev *bridge) > pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu); > } > > -static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type) > + > +void pci_setup_bridge_resources(struct pci_bus *bus, unsigned long type) > { > struct pci_dev *bridge = bus->self; > > @@ -693,12 +694,17 @@ static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type) > pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl); > } > > +void __weak pcibios_setup_bridge(struct pci_bus *bus, unsigned long type) > +{ > + pci_setup_bridge_resources(bus, type); > +} I'm not opposed to adding a pcibios_setup_bridge(), but I would rather do the architected updates in the generic PCI core code instead of down in the pcibios code. In other words, I would rather have this: void pci_setup_bridge(struct pci_bus *bus) { pcibios_setup_bridge(bus, type); pci_setup_bridge_resources(bus, type); } That way the default pcibios hook is empty, showing that by default there's no arch-specific code in this path, and we only have to look at the generic core code to verify that we actually do program the bridge windows. 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 Thu, May 07, 2015 at 05:12:24PM -0500, Bjorn Helgaas wrote: >Hi Gavin, > >[Please run "git log --oneline drivers/pci/setup-bus.c" and observe the >capitalization convention.] > Bjorn, thanks for review. Yeah, it should be something like "PCI: xxxx" for the subject. I'll fix it up in next revision. >On Fri, May 01, 2015 at 04:02:48PM +1000, Gavin Shan wrote: >> Currently, PowerPC PowerNV platform utilizes ppc_md.pcibios_fixup(), >> which is called for once after PCI probing and resource assignment >> are completed, to allocate platform required resources for PCI devices: >> PE#, IO and MMIO mapping, DMA address translation (TCE) table etc. >> Obviously, it's not hotplug friendly. >> >> The patch adds weak function pcibios_setup_bridge(), which is called >> by pci_setup_bridge(). PowerPC PowerNV platform will reuse the function >> to assign above platform required resources to newly added PCI devices, >> in order to support PCI hotplug on PowerPC PowerNV platform. >> >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >> --- >> drivers/pci/setup-bus.c | 12 +++++++++--- >> include/linux/pci.h | 1 + >> 2 files changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c >> index 4fd0cac..a7d0c3c 100644 >> --- a/drivers/pci/setup-bus.c >> +++ b/drivers/pci/setup-bus.c >> @@ -674,7 +674,8 @@ static void pci_setup_bridge_mmio_pref(struct pci_dev *bridge) >> pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu); >> } >> >> -static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type) >> + >> +void pci_setup_bridge_resources(struct pci_bus *bus, unsigned long type) >> { >> struct pci_dev *bridge = bus->self; >> >> @@ -693,12 +694,17 @@ static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type) >> pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl); >> } >> >> +void __weak pcibios_setup_bridge(struct pci_bus *bus, unsigned long type) >> +{ >> + pci_setup_bridge_resources(bus, type); >> +} > >I'm not opposed to adding a pcibios_setup_bridge(), but I would rather do >the architected updates in the generic PCI core code instead of down in the >pcibios code. In other words, I would rather have this: > > void pci_setup_bridge(struct pci_bus *bus) > { > pcibios_setup_bridge(bus, type); > pci_setup_bridge_resources(bus, type); > } > >That way the default pcibios hook is empty, showing that by default there's >no arch-specific code in this path, and we only have to look at the generic >core code to verify that we actually do program the bridge windows. > Ok. I'll change the code accordingly in next revision. Thanks for the suggestion. Thanks, Gavin -- 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
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 4fd0cac..a7d0c3c 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -674,7 +674,8 @@ static void pci_setup_bridge_mmio_pref(struct pci_dev *bridge) pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu); } -static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type) + +void pci_setup_bridge_resources(struct pci_bus *bus, unsigned long type) { struct pci_dev *bridge = bus->self; @@ -693,12 +694,17 @@ static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type) pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl); } +void __weak pcibios_setup_bridge(struct pci_bus *bus, unsigned long type) +{ + pci_setup_bridge_resources(bus, type); +} + void pci_setup_bridge(struct pci_bus *bus) { unsigned long type = IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_PREFETCH; - __pci_setup_bridge(bus, type); + pcibios_setup_bridge(bus, type); } @@ -1467,7 +1473,7 @@ static void pci_bridge_release_resources(struct pci_bus *bus, /* avoiding touch the one without PREF */ if (type & IORESOURCE_PREFETCH) type = IORESOURCE_PREFETCH; - __pci_setup_bridge(bus, type); + pci_setup_bridge_resources(bus, type); /* for next child res under same bridge */ r->flags = old_flags; } diff --git a/include/linux/pci.h b/include/linux/pci.h index 353db8d..68c5ef9 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1175,6 +1175,7 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), void *userdata); int pci_cfg_space_size(struct pci_dev *dev); unsigned char pci_bus_max_busnr(struct pci_bus *bus); +void pci_setup_bridge_resources(struct pci_bus *bus, unsigned long type); void pci_setup_bridge(struct pci_bus *bus); resource_size_t pcibios_window_alignment(struct pci_bus *bus, unsigned long type);
Currently, PowerPC PowerNV platform utilizes ppc_md.pcibios_fixup(), which is called for once after PCI probing and resource assignment are completed, to allocate platform required resources for PCI devices: PE#, IO and MMIO mapping, DMA address translation (TCE) table etc. Obviously, it's not hotplug friendly. The patch adds weak function pcibios_setup_bridge(), which is called by pci_setup_bridge(). PowerPC PowerNV platform will reuse the function to assign above platform required resources to newly added PCI devices, in order to support PCI hotplug on PowerPC PowerNV platform. Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- drivers/pci/setup-bus.c | 12 +++++++++--- include/linux/pci.h | 1 + 2 files changed, 10 insertions(+), 3 deletions(-)