diff mbox

[1/3] PCI: Add hooks for bus/bridge-specific fixups

Message ID 20180619141700.7842-2-hch@lst.de (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Christoph Hellwig June 19, 2018, 2:16 p.m. UTC 
From: "Wesley W. Terpstra" <wesley@sifive.com>

There is currently no way for a PCIe bridge to impose constraints on
devices added to it.  For example, the Xilinx PCIe host bridge only
supports 32-bit physical addresses (due to a limitation on the AXI
port's address width).  Thus, even devices that claim to support 64-bit
DMA addresses must be restricted to 32-bit addresses when attached to
this host controller.

This patch adds at "add_dev" hook to pci_ops that allows the PCI
infastructure to interpose when adding a device, which in the case of
the Xilinx driver will be used to restrict the address ranges used for
DMA.

Signed-off-by: Wesley W. Terpstra <wesley@sifive.com>
[hch: simplified the prototype]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/probe.c | 3 +++
 include/linux/pci.h | 1 +
 2 files changed, 4 insertions(+)

Comments

Arnd Bergmann June 19, 2018, 2:31 p.m. UTC | #1 
On Tue, Jun 19, 2018 at 4:16 PM, Christoph Hellwig <hch@lst.de> wrote:
> From: "Wesley W. Terpstra" <wesley@sifive.com>

> index 340029b2fb38..ea9609fc44fc 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -662,6 +662,7 @@ static inline int pcibios_err_to_errno(int err)
>  struct pci_ops {
>         int (*add_bus)(struct pci_bus *bus);
>         void (*remove_bus)(struct pci_bus *bus);
> +       void (*add_dev)(struct pci_dev *dev, struct pci_bus *bus);
>         void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
>         int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
>         int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);

We are a bit inconsistent about adding callbacks to pci_ops or pci_host_bridge.
I would argue this should be part of pci_host_bridge, and the other two should
probably be there as well, or possibly moved into a separate
'pci_host_bridge_ops'
structure referenced from pci_host_bridge along with the other callbacks there.

One of the things we discussed in the past (but never implemented) is that
many of the current '__weak' functions in the PCI core can be turned into
host_bridge specific operations.

Some config space operations by contrast are currently shared across host
bridge drivers (see pci_bus_set_ops, or drivers/pci/ecam.c) and probably
better left out of the host bridge.

      Arnd
Christoph Hellwig June 19, 2018, 5:32 p.m. UTC | #2 
On Tue, Jun 19, 2018 at 04:31:06PM +0200, Arnd Bergmann wrote:
> We are a bit inconsistent about adding callbacks to pci_ops or pci_host_bridge.
> I would argue this should be part of pci_host_bridge, and the other two should
> probably be there as well, or possibly moved into a separate
> 'pci_host_bridge_ops'
> structure referenced from pci_host_bridge along with the other callbacks there.

How do we find the proper pci_host_bridge from an arbitrary
device deep down the hierachy below it?
Sinan Kaya June 19, 2018, 6:48 p.m. UTC | #3 
On Tue, Jun 19, 2018 at 07:32:40PM +0200, Christoph Hellwig wrote:
> On Tue, Jun 19, 2018 at 04:31:06PM +0200, Arnd Bergmann wrote:
> > We are a bit inconsistent about adding callbacks to pci_ops or pci_host_bridge.
> > I would argue this should be part of pci_host_bridge, and the other two should
> > probably be there as well, or possibly moved into a separate
> > 'pci_host_bridge_ops'
> > structure referenced from pci_host_bridge along with the other callbacks there.
> 
> How do we find the proper pci_host_bridge from an arbitrary
> device deep down the hierachy below it?

pci_get_host_bridge_device() should do the work.
diff mbox

Patch

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac876e32de4b..978e684cba2c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2331,6 +2331,9 @@  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	/* Set up MSI IRQ domain */
 	pci_set_msi_domain(dev);
 
+	if (bus->ops->add_dev)
+		bus->ops->add_dev(dev, bus);
+
 	/* Notifier could use PCI capabilities */
 	dev->match_driver = false;
 	ret = device_add(&dev->dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 340029b2fb38..ea9609fc44fc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -662,6 +662,7 @@  static inline int pcibios_err_to_errno(int err)
 struct pci_ops {
 	int (*add_bus)(struct pci_bus *bus);
 	void (*remove_bus)(struct pci_bus *bus);
+	void (*add_dev)(struct pci_dev *dev, struct pci_bus *bus);
 	void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
 	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
 	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);