add support for Xilinx PCIe root ports on RISC-V v3

add support for Xilinx PCIe root ports on RISC-V v3

[Christoph Hellwig]

Hi all,

this series with patches originally from Palmer and Wesley adds support
for the pcie-xilinx host driver on RISC-V boards.  The interesting part
about that is that the IP blocks is limited to 32-bit DMA internally,
which didn't seem to be an issue with the existing users, but shows
up easily with the Sifive RISC-V boards that have physical memory
wired up above 4G.

Note that patches 1 and 2 depend on changes in the dma-mapping tree
to add the bus_dma_mask field to struct device and would have to merge
through the dma-mapping tree or a shared stable branch.  Patch 3 could
be merged independently.

Changes since v2:
 - rename the add_dev callback to add_device and allow it to return an
   error code (to allow it to replace pcibios_add_device in the future)

Changes since v1:
 - move the add_dev method to struct pci_host_bridge
 - use the new bus_dma_mask field

add support for Xilinx PCIe root ports on RISC-V v3

[Christoph Hellwig]

Hi all,

this series with patches originally from Palmer and Wesley adds support
for the pcie-xilinx host driver on RISC-V boards.  The interesting part
about that is that the IP blocks is limited to 32-bit DMA internally,
which didn't seem to be an issue with the existing users, but shows
up easily with the Sifive RISC-V boards that have physical memory
wired up above 4G.

Note that patches 1 and 2 depend on changes in the dma-mapping tree
to add the bus_dma_mask field to struct device and would have to merge
through the dma-mapping tree or a shared stable branch.  Patch 3 could
be merged independently.

Changes since v2:
 - rename the add_dev callback to add_device and allow it to return an
   error code (to allow it to replace pcibios_add_device in the future)

Changes since v1:
 - move the add_dev method to struct pci_host_bridge
 - use the new bus_dma_mask field

[PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Christoph Hellwig]

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 a "add_device" method  to struct pci_host_bridge that
allows the host driver to act upon acting adding devices.

Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/probe.c | 6 ++++++
 include/linux/pci.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac876e32de4b..452190fb05e7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2290,6 +2290,7 @@ static void pci_set_msi_domain(struct pci_dev *dev)
 
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 {
+	struct pci_host_bridge *host = pci_find_host_bridge(bus);
 	int ret;
 
 	pci_configure_device(dev);
@@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	ret = pcibios_add_device(dev);
 	WARN_ON(ret < 0);
 
+	if (host->add_device) {
+		ret = host->add_device(dev);
+		WARN_ON(ret < 0);
+	}
+
 	/* Set up MSI IRQ domain */
 	pci_set_msi_domain(dev);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index abd5d5e17aee..1524adbb30ab 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -485,6 +485,7 @@ struct pci_host_bridge {
 			resource_size_t start,
 			resource_size_t size,
 			resource_size_t align);
+	int (*add_device)(struct pci_dev *dev);
 	unsigned long	private[0] ____cacheline_aligned;
 };
 
-- 
2.18.0

[PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Christoph Hellwig]

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 a "add_device" method  to struct pci_host_bridge that
allows the host driver to act upon acting adding devices.

Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/probe.c | 6 ++++++
 include/linux/pci.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ac876e32de4b..452190fb05e7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2290,6 +2290,7 @@ static void pci_set_msi_domain(struct pci_dev *dev)
 
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 {
+	struct pci_host_bridge *host = pci_find_host_bridge(bus);
 	int ret;
 
 	pci_configure_device(dev);
@@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	ret = pcibios_add_device(dev);
 	WARN_ON(ret < 0);
 
+	if (host->add_device) {
+		ret = host->add_device(dev);
+		WARN_ON(ret < 0);
+	}
+
 	/* Set up MSI IRQ domain */
 	pci_set_msi_domain(dev);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index abd5d5e17aee..1524adbb30ab 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -485,6 +485,7 @@ struct pci_host_bridge {
 			resource_size_t start,
 			resource_size_t size,
 			resource_size_t align);
+	int (*add_device)(struct pci_dev *dev);
 	unsigned long	private[0] ____cacheline_aligned;
 };
 
-- 
2.18.0

[PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Christoph Hellwig]

This PCIe bridge only has a 32 bit bus master interface, thus truncating
the DMA capability of all PCIe devices attached beneath it. This caps
the child device capability so that these devices work on systems with
physical memory beyond the 4GiB threshold.

Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/pcie-xilinx.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index 7b1389d8e2a5..ccfd91e0515f 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -197,6 +197,16 @@ static void __iomem *xilinx_pcie_map_bus(struct pci_bus *bus,
 	return port->reg_base + relbus + where;
 }
 
+/*
+ * This PCIe bridge only has a 32 bit bus master interface, thus truncating
+ * the DMA capability of all PCIe devices attached beneath it.
+ */
+static int xilinx_pcie_add_device(struct pci_dev *pdev)
+{
+	pdev->dev.bus_dma_mask = DMA_BIT_MASK(32);
+	return 0;
+}
+
 /* PCIe operations */
 static struct pci_ops xilinx_pcie_ops = {
 	.map_bus = xilinx_pcie_map_bus,
@@ -665,6 +675,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 	bridge->ops = &xilinx_pcie_ops;
 	bridge->map_irq = of_irq_parse_and_map_pci;
 	bridge->swizzle_irq = pci_common_swizzle;
+	bridge->add_device = xilinx_pcie_add_device;
 
 #ifdef CONFIG_PCI_MSI
 	xilinx_pcie_msi_chip.dev = dev;
-- 
2.18.0

[PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Christoph Hellwig]

This PCIe bridge only has a 32 bit bus master interface, thus truncating
the DMA capability of all PCIe devices attached beneath it. This caps
the child device capability so that these devices work on systems with
physical memory beyond the 4GiB threshold.

Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/pcie-xilinx.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index 7b1389d8e2a5..ccfd91e0515f 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -197,6 +197,16 @@ static void __iomem *xilinx_pcie_map_bus(struct pci_bus *bus,
 	return port->reg_base + relbus + where;
 }
 
+/*
+ * This PCIe bridge only has a 32 bit bus master interface, thus truncating
+ * the DMA capability of all PCIe devices attached beneath it.
+ */
+static int xilinx_pcie_add_device(struct pci_dev *pdev)
+{
+	pdev->dev.bus_dma_mask = DMA_BIT_MASK(32);
+	return 0;
+}
+
 /* PCIe operations */
 static struct pci_ops xilinx_pcie_ops = {
 	.map_bus = xilinx_pcie_map_bus,
@@ -665,6 +675,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 	bridge->ops = &xilinx_pcie_ops;
 	bridge->map_irq = of_irq_parse_and_map_pci;
 	bridge->swizzle_irq = pci_common_swizzle;
+	bridge->add_device = xilinx_pcie_add_device;
 
 #ifdef CONFIG_PCI_MSI
 	xilinx_pcie_msi_chip.dev = dev;
-- 
2.18.0

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
port will need to be able to enable PCIE_XILINX in order to have PCIe
support.

This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
depends on OF, which I believe is the only true dependency.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: switch to OF instead of OF_PCI now that the latter is gone]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index cc9fa02d32a0..0f7ce5eaeac8 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -102,7 +102,7 @@ config PCI_HOST_GENERIC
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
+	depends on OF || COMPILE_TEST
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.18.0

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
port will need to be able to enable PCIE_XILINX in order to have PCIe
support.

This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
depends on OF, which I believe is the only true dependency.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: switch to OF instead of OF_PCI now that the latter is gone]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index cc9fa02d32a0..0f7ce5eaeac8 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -102,7 +102,7 @@ config PCI_HOST_GENERIC
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
+	depends on OF || COMPILE_TEST
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.18.0

Re: [PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Wesley Terpstra]

FYI, This Xilinx PCIe IP 32-bit cap only applies to SOME instances of
the IP. The Ultrascale+ version of Xilinx PCIe hard IP does support
64-bit or 32-bit. The Virtex7 version only supports 32-bit. The
pcie-xilinx driver woks with both of these root complexes. So probably
there should be a conditional hook in the DTS that triggers the
work-around behaviour.

On Sat, Aug 4, 2018 at 3:14 AM, Christoph Hellwig <hch@lst.de> wrote:
> This PCIe bridge only has a 32 bit bus master interface, thus truncating
> the DMA capability of all PCIe devices attached beneath it. This caps
> the child device capability so that these devices work on systems with
> physical memory beyond the 4GiB threshold.
>
> Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/controller/pcie-xilinx.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index 7b1389d8e2a5..ccfd91e0515f 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -197,6 +197,16 @@ static void __iomem *xilinx_pcie_map_bus(struct pci_bus *bus,
>         return port->reg_base + relbus + where;
>  }
>
> +/*
> + * This PCIe bridge only has a 32 bit bus master interface, thus truncating
> + * the DMA capability of all PCIe devices attached beneath it.
> + */
> +static int xilinx_pcie_add_device(struct pci_dev *pdev)
> +{
> +       pdev->dev.bus_dma_mask = DMA_BIT_MASK(32);
> +       return 0;
> +}
> +
>  /* PCIe operations */
>  static struct pci_ops xilinx_pcie_ops = {
>         .map_bus = xilinx_pcie_map_bus,
> @@ -665,6 +675,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>         bridge->ops = &xilinx_pcie_ops;
>         bridge->map_irq = of_irq_parse_and_map_pci;
>         bridge->swizzle_irq = pci_common_swizzle;
> +       bridge->add_device = xilinx_pcie_add_device;
>
>  #ifdef CONFIG_PCI_MSI
>         xilinx_pcie_msi_chip.dev = dev;
> --
> 2.18.0
>

[PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Wesley Terpstra]

FYI, This Xilinx PCIe IP 32-bit cap only applies to SOME instances of
the IP. The Ultrascale+ version of Xilinx PCIe hard IP does support
64-bit or 32-bit. The Virtex7 version only supports 32-bit. The
pcie-xilinx driver woks with both of these root complexes. So probably
there should be a conditional hook in the DTS that triggers the
work-around behaviour.

On Sat, Aug 4, 2018 at 3:14 AM, Christoph Hellwig <hch@lst.de> wrote:
> This PCIe bridge only has a 32 bit bus master interface, thus truncating
> the DMA capability of all PCIe devices attached beneath it. This caps
> the child device capability so that these devices work on systems with
> physical memory beyond the 4GiB threshold.
>
> Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/controller/pcie-xilinx.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index 7b1389d8e2a5..ccfd91e0515f 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -197,6 +197,16 @@ static void __iomem *xilinx_pcie_map_bus(struct pci_bus *bus,
>         return port->reg_base + relbus + where;
>  }
>
> +/*
> + * This PCIe bridge only has a 32 bit bus master interface, thus truncating
> + * the DMA capability of all PCIe devices attached beneath it.
> + */
> +static int xilinx_pcie_add_device(struct pci_dev *pdev)
> +{
> +       pdev->dev.bus_dma_mask = DMA_BIT_MASK(32);
> +       return 0;
> +}
> +
>  /* PCIe operations */
>  static struct pci_ops xilinx_pcie_ops = {
>         .map_bus = xilinx_pcie_map_bus,
> @@ -665,6 +675,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>         bridge->ops = &xilinx_pcie_ops;
>         bridge->map_irq = of_irq_parse_and_map_pci;
>         bridge->swizzle_irq = pci_common_swizzle;
> +       bridge->add_device = xilinx_pcie_add_device;
>
>  #ifdef CONFIG_PCI_MSI
>         xilinx_pcie_msi_chip.dev = dev;
> --
> 2.18.0
>

Re: [PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Lorenzo Pieralisi]

On Sat, Aug 04, 2018 at 12:14:02PM +0200, Christoph Hellwig wrote:
> There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
> architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
> and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
> port will need to be able to enable PCIE_XILINX in order to have PCIe
> support.
> 
> This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
> depends on OF, which I believe is the only true dependency.
> 
> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> [hch: switch to OF instead of OF_PCI now that the latter is gone]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/controller/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I have applied it to pci/controller/misc for v4.19, even though I am
not sure Bjorn will have time to put into -next and send it for
the upcoming merge window, we shall try.

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index cc9fa02d32a0..0f7ce5eaeac8 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -102,7 +102,7 @@ config PCI_HOST_GENERIC
>  
>  config PCIE_XILINX
>  	bool "Xilinx AXI PCIe host bridge support"
> -	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
> +	depends on OF || COMPILE_TEST
>  	help
>  	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
>  	  Host Bridge driver.
> -- 
> 2.18.0
> 

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Lorenzo Pieralisi]

On Sat, Aug 04, 2018 at 12:14:02PM +0200, Christoph Hellwig wrote:
> There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
> architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
> and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
> port will need to be able to enable PCIE_XILINX in order to have PCIe
> support.
> 
> This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
> depends on OF, which I believe is the only true dependency.
> 
> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> [hch: switch to OF instead of OF_PCI now that the latter is gone]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/controller/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I have applied it to pci/controller/misc for v4.19, even though I am
not sure Bjorn will have time to put into -next and send it for
the upcoming merge window, we shall try.

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index cc9fa02d32a0..0f7ce5eaeac8 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -102,7 +102,7 @@ config PCI_HOST_GENERIC
>  
>  config PCIE_XILINX
>  	bool "Xilinx AXI PCIe host bridge support"
> -	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
> +	depends on OF || COMPILE_TEST
>  	help
>  	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
>  	  Host Bridge driver.
> -- 
> 2.18.0
> 

Re: [PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Lorenzo Pieralisi]

On Sat, Aug 04, 2018 at 12:14:00PM +0200, Christoph Hellwig wrote:
> 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 a "add_device" method  to struct pci_host_bridge that
> allows the host driver to act upon acting adding devices.
> 
> Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/probe.c | 6 ++++++
>  include/linux/pci.h | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index ac876e32de4b..452190fb05e7 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2290,6 +2290,7 @@ static void pci_set_msi_domain(struct pci_dev *dev)
>  
>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
> +	struct pci_host_bridge *host = pci_find_host_bridge(bus);
>  	int ret;
>  
>  	pci_configure_device(dev);
> @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	ret = pcibios_add_device(dev);
>  	WARN_ON(ret < 0);
>  
> +	if (host->add_device) {
> +		ret = host->add_device(dev);
> +		WARN_ON(ret < 0);
> +	}

This looks fine; we could go a step further and make the hunk above
the default (weak) implementation of pcibios_add_device() that is
currently a NOP returning 0, I will remove it for v4.20.

I am fine with the patch as-is (even though I am not a big fan of
those WARN_ON but that's not a problem related to your patch, as you
said we are *not* handling the ret value in the current mainline).

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> +
>  	/* Set up MSI IRQ domain */
>  	pci_set_msi_domain(dev);
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index abd5d5e17aee..1524adbb30ab 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -485,6 +485,7 @@ struct pci_host_bridge {
>  			resource_size_t start,
>  			resource_size_t size,
>  			resource_size_t align);
> +	int (*add_device)(struct pci_dev *dev);
>  	unsigned long	private[0] ____cacheline_aligned;
>  };
>  
> -- 
> 2.18.0
> 

[PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Lorenzo Pieralisi]

On Sat, Aug 04, 2018 at 12:14:00PM +0200, Christoph Hellwig wrote:
> 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 a "add_device" method  to struct pci_host_bridge that
> allows the host driver to act upon acting adding devices.
> 
> Based on an earlier patch from Wesley W. Terpstra <wesley@sifive.com>.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/probe.c | 6 ++++++
>  include/linux/pci.h | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index ac876e32de4b..452190fb05e7 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2290,6 +2290,7 @@ static void pci_set_msi_domain(struct pci_dev *dev)
>  
>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  {
> +	struct pci_host_bridge *host = pci_find_host_bridge(bus);
>  	int ret;
>  
>  	pci_configure_device(dev);
> @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	ret = pcibios_add_device(dev);
>  	WARN_ON(ret < 0);
>  
> +	if (host->add_device) {
> +		ret = host->add_device(dev);
> +		WARN_ON(ret < 0);
> +	}

This looks fine; we could go a step further and make the hunk above
the default (weak) implementation of pcibios_add_device() that is
currently a NOP returning 0, I will remove it for v4.20.

I am fine with the patch as-is (even though I am not a big fan of
those WARN_ON but that's not a problem related to your patch, as you
said we are *not* handling the ret value in the current mainline).

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> +
>  	/* Set up MSI IRQ domain */
>  	pci_set_msi_domain(dev);
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index abd5d5e17aee..1524adbb30ab 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -485,6 +485,7 @@ struct pci_host_bridge {
>  			resource_size_t start,
>  			resource_size_t size,
>  			resource_size_t align);
> +	int (*add_device)(struct pci_dev *dev);
>  	unsigned long	private[0] ____cacheline_aligned;
>  };
>  
> -- 
> 2.18.0
> 

Re: [PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Christoph Hellwig]

On Mon, Aug 06, 2018 at 12:23:10PM +0100, Lorenzo Pieralisi wrote:
> >  	pci_configure_device(dev);
> > @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> >  	ret = pcibios_add_device(dev);
> >  	WARN_ON(ret < 0);
> >  
> > +	if (host->add_device) {
> > +		ret = host->add_device(dev);
> > +		WARN_ON(ret < 0);
> > +	}
> 
> This looks fine; we could go a step further and make the hunk above
> the default (weak) implementation of pcibios_add_device() that is
> currently a NOP returning 0, I will remove it for v4.20.

I'd love to see pcibios_add_device go away entirely.  But I wonder how to
get setup the pci_host_bridge pointer for the remaining architectures that
still implement it.  I did look for any easy way but couldn't find one.
But then I don't really know this area of the PCI code too well.

[PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Christoph Hellwig]

On Mon, Aug 06, 2018 at 12:23:10PM +0100, Lorenzo Pieralisi wrote:
> >  	pci_configure_device(dev);
> > @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> >  	ret = pcibios_add_device(dev);
> >  	WARN_ON(ret < 0);
> >  
> > +	if (host->add_device) {
> > +		ret = host->add_device(dev);
> > +		WARN_ON(ret < 0);
> > +	}
> 
> This looks fine; we could go a step further and make the hunk above
> the default (weak) implementation of pcibios_add_device() that is
> currently a NOP returning 0, I will remove it for v4.20.

I'd love to see pcibios_add_device go away entirely.  But I wonder how to
get setup the pci_host_bridge pointer for the remaining architectures that
still implement it.  I did look for any easy way but couldn't find one.
But then I don't really know this area of the PCI code too well.

Re: [PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Christoph Hellwig]

On Sun, Aug 05, 2018 at 01:02:58PM -0700, Wesley Terpstra wrote:
> FYI, This Xilinx PCIe IP 32-bit cap only applies to SOME instances of
> the IP. The Ultrascale+ version of Xilinx PCIe hard IP does support
> 64-bit or 32-bit. The Virtex7 version only supports 32-bit. The
> pcie-xilinx driver woks with both of these root complexes. So probably
> there should be a conditional hook in the DTS that triggers the
> work-around behaviour.

Either we'll need to able to detect which version we use based on
registrs, or we will indeed need firmware information.

Note that we already have the mechanism for firmware directed dma limits
in place, it is called the dma-ranges DT property.  If we can get the
SiFive firmware to set it up properly the RISC-V swiotlb code will
just do the right thing.

[PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Christoph Hellwig]

On Sun, Aug 05, 2018 at 01:02:58PM -0700, Wesley Terpstra wrote:
> FYI, This Xilinx PCIe IP 32-bit cap only applies to SOME instances of
> the IP. The Ultrascale+ version of Xilinx PCIe hard IP does support
> 64-bit or 32-bit. The Virtex7 version only supports 32-bit. The
> pcie-xilinx driver woks with both of these root complexes. So probably
> there should be a conditional hook in the DTS that triggers the
> work-around behaviour.

Either we'll need to able to detect which version we use based on
registrs, or we will indeed need firmware information.

Note that we already have the mechanism for firmware directed dma limits
in place, it is called the dma-ranges DT property.  If we can get the
SiFive firmware to set it up properly the RISC-V swiotlb code will
just do the right thing.

Re: [PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Lorenzo Pieralisi]

On Mon, Aug 06, 2018 at 02:35:27PM +0200, Christoph Hellwig wrote:
> On Sun, Aug 05, 2018 at 01:02:58PM -0700, Wesley Terpstra wrote:
> > FYI, This Xilinx PCIe IP 32-bit cap only applies to SOME instances of
> > the IP. The Ultrascale+ version of Xilinx PCIe hard IP does support
> > 64-bit or 32-bit. The Virtex7 version only supports 32-bit. The
> > pcie-xilinx driver woks with both of these root complexes. So probably
> > there should be a conditional hook in the DTS that triggers the
> > work-around behaviour.
> 
> Either we'll need to able to detect which version we use based on
> registrs, or we will indeed need firmware information.
> 
> Note that we already have the mechanism for firmware directed dma limits
> in place, it is called the dma-ranges DT property.  If we can get the
> SiFive firmware to set it up properly the RISC-V swiotlb code will
> just do the right thing.

It would do the right thing without patches 1 and 2 (I would
consider merging patch 1 anyway - see pcibios_add_device()
removal).

It seems to me that the best course of action consists in patching
firmware, that would remove the need for patch 2 (I will merge patch
1 anyway - even if that's for a "different" purpose, see
pcibios_add_device() removal), please let us know.

Thanks,
Lorenzo

[PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Lorenzo Pieralisi]

On Mon, Aug 06, 2018 at 02:35:27PM +0200, Christoph Hellwig wrote:
> On Sun, Aug 05, 2018 at 01:02:58PM -0700, Wesley Terpstra wrote:
> > FYI, This Xilinx PCIe IP 32-bit cap only applies to SOME instances of
> > the IP. The Ultrascale+ version of Xilinx PCIe hard IP does support
> > 64-bit or 32-bit. The Virtex7 version only supports 32-bit. The
> > pcie-xilinx driver woks with both of these root complexes. So probably
> > there should be a conditional hook in the DTS that triggers the
> > work-around behaviour.
> 
> Either we'll need to able to detect which version we use based on
> registrs, or we will indeed need firmware information.
> 
> Note that we already have the mechanism for firmware directed dma limits
> in place, it is called the dma-ranges DT property.  If we can get the
> SiFive firmware to set it up properly the RISC-V swiotlb code will
> just do the right thing.

It would do the right thing without patches 1 and 2 (I would
consider merging patch 1 anyway - see pcibios_add_device()
removal).

It seems to me that the best course of action consists in patching
firmware, that would remove the need for patch 2 (I will merge patch
1 anyway - even if that's for a "different" purpose, see
pcibios_add_device() removal), please let us know.

Thanks,
Lorenzo

Re: [PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Arnd Bergmann]

On Mon, Aug 6, 2018 at 2:25 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, Aug 06, 2018 at 12:23:10PM +0100, Lorenzo Pieralisi wrote:
> > >     pci_configure_device(dev);
> > > @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> > >     ret = pcibios_add_device(dev);
> > >     WARN_ON(ret < 0);
> > >
> > > +   if (host->add_device) {
> > > +           ret = host->add_device(dev);
> > > +           WARN_ON(ret < 0);
> > > +   }
> >
> > This looks fine; we could go a step further and make the hunk above
> > the default (weak) implementation of pcibios_add_device() that is
> > currently a NOP returning 0, I will remove it for v4.20.
>
> I'd love to see pcibios_add_device go away entirely.  But I wonder how to
> get setup the pci_host_bridge pointer for the remaining architectures that
> still implement it.  I did look for any easy way but couldn't find one.
> But then I don't really know this area of the PCI code too well.

If the platform doesn't already have a pointer to its pci_host_bridge
structure, it can call pci_find_host_bridge(bus->dev) to get it.

Ideally, platforms (either arch specific code or drivers/pci/controller)
should call pci_alloc_host_bridge()/pci_register_host_bridge()
instead of the traditional pci_create_root_bus() that is less flexible.
That way, the driver specific structure can get allocated together with
the host bridge.

         Arnd

[PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Arnd Bergmann]

On Mon, Aug 6, 2018 at 2:25 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, Aug 06, 2018 at 12:23:10PM +0100, Lorenzo Pieralisi wrote:
> > >     pci_configure_device(dev);
> > > @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> > >     ret = pcibios_add_device(dev);
> > >     WARN_ON(ret < 0);
> > >
> > > +   if (host->add_device) {
> > > +           ret = host->add_device(dev);
> > > +           WARN_ON(ret < 0);
> > > +   }
> >
> > This looks fine; we could go a step further and make the hunk above
> > the default (weak) implementation of pcibios_add_device() that is
> > currently a NOP returning 0, I will remove it for v4.20.
>
> I'd love to see pcibios_add_device go away entirely.  But I wonder how to
> get setup the pci_host_bridge pointer for the remaining architectures that
> still implement it.  I did look for any easy way but couldn't find one.
> But then I don't really know this area of the PCI code too well.

If the platform doesn't already have a pointer to its pci_host_bridge
structure, it can call pci_find_host_bridge(bus->dev) to get it.

Ideally, platforms (either arch specific code or drivers/pci/controller)
should call pci_alloc_host_bridge()/pci_register_host_bridge()
instead of the traditional pci_create_root_bus() that is less flexible.
That way, the driver specific structure can get allocated together with
the host bridge.

         Arnd

Re: [PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Lorenzo Pieralisi]

On Mon, Aug 06, 2018 at 03:54:13PM +0200, Arnd Bergmann wrote:
> On Mon, Aug 6, 2018 at 2:25 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Mon, Aug 06, 2018 at 12:23:10PM +0100, Lorenzo Pieralisi wrote:
> > > >     pci_configure_device(dev);
> > > > @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> > > >     ret = pcibios_add_device(dev);
> > > >     WARN_ON(ret < 0);
> > > >
> > > > +   if (host->add_device) {
> > > > +           ret = host->add_device(dev);
> > > > +           WARN_ON(ret < 0);
> > > > +   }
> > >
> > > This looks fine; we could go a step further and make the hunk above
> > > the default (weak) implementation of pcibios_add_device() that is
> > > currently a NOP returning 0, I will remove it for v4.20.
> >
> > I'd love to see pcibios_add_device go away entirely.  But I wonder how to
> > get setup the pci_host_bridge pointer for the remaining architectures that
> > still implement it.  I did look for any easy way but couldn't find one.
> > But then I don't really know this area of the PCI code too well.
> 
> If the platform doesn't already have a pointer to its pci_host_bridge
> structure, it can call pci_find_host_bridge(bus->dev) to get it.
> 
> Ideally, platforms (either arch specific code or drivers/pci/controller)
> should call pci_alloc_host_bridge()/pci_register_host_bridge()
> instead of the traditional pci_create_root_bus() that is less flexible.
> That way, the driver specific structure can get allocated together with
> the host bridge.

Yes that's basically the gist, it should be a matter of identifying code
that creates the root bus (explicitly or implicitly - ie
pci_scan_root_bus()) and patching it up, probably something to be done
(and moved into -next) early -rc* since it is something that can easily
break the world if we miss some code paths. I will have a look into
that and can take this patch for that series if it does not go into
v4.19.

Thanks,
Lorenzo

[PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Lorenzo Pieralisi]

On Mon, Aug 06, 2018 at 03:54:13PM +0200, Arnd Bergmann wrote:
> On Mon, Aug 6, 2018 at 2:25 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Mon, Aug 06, 2018 at 12:23:10PM +0100, Lorenzo Pieralisi wrote:
> > > >     pci_configure_device(dev);
> > > > @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> > > >     ret = pcibios_add_device(dev);
> > > >     WARN_ON(ret < 0);
> > > >
> > > > +   if (host->add_device) {
> > > > +           ret = host->add_device(dev);
> > > > +           WARN_ON(ret < 0);
> > > > +   }
> > >
> > > This looks fine; we could go a step further and make the hunk above
> > > the default (weak) implementation of pcibios_add_device() that is
> > > currently a NOP returning 0, I will remove it for v4.20.
> >
> > I'd love to see pcibios_add_device go away entirely.  But I wonder how to
> > get setup the pci_host_bridge pointer for the remaining architectures that
> > still implement it.  I did look for any easy way but couldn't find one.
> > But then I don't really know this area of the PCI code too well.
> 
> If the platform doesn't already have a pointer to its pci_host_bridge
> structure, it can call pci_find_host_bridge(bus->dev) to get it.
> 
> Ideally, platforms (either arch specific code or drivers/pci/controller)
> should call pci_alloc_host_bridge()/pci_register_host_bridge()
> instead of the traditional pci_create_root_bus() that is less flexible.
> That way, the driver specific structure can get allocated together with
> the host bridge.

Yes that's basically the gist, it should be a matter of identifying code
that creates the root bus (explicitly or implicitly - ie
pci_scan_root_bus()) and patching it up, probably something to be done
(and moved into -next) early -rc* since it is something that can easily
break the world if we miss some code paths. I will have a look into
that and can take this patch for that series if it does not go into
v4.19.

Thanks,
Lorenzo

Re: [PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Christoph Hellwig]

On Mon, Aug 06, 2018 at 02:40:43PM +0100, Lorenzo Pieralisi wrote:
> It would do the right thing without patches 1 and 2 (I would
> consider merging patch 1 anyway - see pcibios_add_device()
> removal).
> 
> It seems to me that the best course of action consists in patching
> firmware, that would remove the need for patch 2 (I will merge patch
> 1 anyway - even if that's for a "different" purpose, see
> pcibios_add_device() removal), please let us know.

Sounds good to me.  Patch 1 also doesn't even depend on dma-mapping
bits so feel free to pull it in when you think is the best time.

[PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Christoph Hellwig]

On Mon, Aug 06, 2018 at 02:40:43PM +0100, Lorenzo Pieralisi wrote:
> It would do the right thing without patches 1 and 2 (I would
> consider merging patch 1 anyway - see pcibios_add_device()
> removal).
> 
> It seems to me that the best course of action consists in patching
> firmware, that would remove the need for patch 2 (I will merge patch
> 1 anyway - even if that's for a "different" purpose, see
> pcibios_add_device() removal), please let us know.

Sounds good to me.  Patch 1 also doesn't even depend on dma-mapping
bits so feel free to pull it in when you think is the best time.

Re: [PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Wesley Terpstra]

On Mon, Aug 6, 2018 at 5:35 AM, Christoph Hellwig <hch@lst.de> wrote:
> Note that we already have the mechanism for firmware directed dma limits
> in place, it is called the dma-ranges DT property.  If we can get the
> SiFive firmware to set it up properly the RISC-V swiotlb code will
> just do the right thing.

Does this mean we only need to set the dma-ranges property inside the
pci DTS node? No changes to the driver needed?

[PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Wesley Terpstra]

On Mon, Aug 6, 2018 at 5:35 AM, Christoph Hellwig <hch@lst.de> wrote:
> Note that we already have the mechanism for firmware directed dma limits
> in place, it is called the dma-ranges DT property.  If we can get the
> SiFive firmware to set it up properly the RISC-V swiotlb code will
> just do the right thing.

Does this mean we only need to set the dma-ranges property inside the
pci DTS node? No changes to the driver needed?

Re: [PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Christoph Hellwig]

On Mon, Aug 06, 2018 at 09:21:40AM -0700, Wesley Terpstra wrote:
> On Mon, Aug 6, 2018 at 5:35 AM, Christoph Hellwig <hch@lst.de> wrote:
> > Note that we already have the mechanism for firmware directed dma limits
> > in place, it is called the dma-ranges DT property.  If we can get the
> > SiFive firmware to set it up properly the RISC-V swiotlb code will
> > just do the right thing.
> 
> Does this mean we only need to set the dma-ranges property inside the
> pci DTS node? No changes to the driver needed?

The code looks at the DT parent of the PCI bridge device.  Take a look
at drivers/pci/pci-driver.c:pci_dma_configure() and
drivers/of/device.c:of_dma_configure() in 4.18-rc (for older kernels
the involved functions are slightly different, but the functionality
is the same).

[PATCH 2/3] PCI/xilinx: Work-around for hardware DMA limit (32 bits)

[Christoph Hellwig]

On Mon, Aug 06, 2018 at 09:21:40AM -0700, Wesley Terpstra wrote:
> On Mon, Aug 6, 2018 at 5:35 AM, Christoph Hellwig <hch@lst.de> wrote:
> > Note that we already have the mechanism for firmware directed dma limits
> > in place, it is called the dma-ranges DT property.  If we can get the
> > SiFive firmware to set it up properly the RISC-V swiotlb code will
> > just do the right thing.
> 
> Does this mean we only need to set the dma-ranges property inside the
> pci DTS node? No changes to the driver needed?

The code looks at the DT parent of the PCI bridge device.  Take a look
at drivers/pci/pci-driver.c:pci_dma_configure() and
drivers/of/device.c:of_dma_configure() in 4.18-rc (for older kernels
the involved functions are slightly different, but the functionality
is the same).

Re: [PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Arnd Bergmann]

On Mon, Aug 6, 2018 at 4:56 PM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Mon, Aug 06, 2018 at 03:54:13PM +0200, Arnd Bergmann wrote:
> > On Mon, Aug 6, 2018 at 2:25 PM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > On Mon, Aug 06, 2018 at 12:23:10PM +0100, Lorenzo Pieralisi wrote:
> > > > >     pci_configure_device(dev);
> > > > > @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> > > > >     ret = pcibios_add_device(dev);
> > > > >     WARN_ON(ret < 0);
> > > > >
> > > > > +   if (host->add_device) {
> > > > > +           ret = host->add_device(dev);
> > > > > +           WARN_ON(ret < 0);
> > > > > +   }
> > > >
> > > > This looks fine; we could go a step further and make the hunk above
> > > > the default (weak) implementation of pcibios_add_device() that is
> > > > currently a NOP returning 0, I will remove it for v4.20.
> > >
> > > I'd love to see pcibios_add_device go away entirely.  But I wonder how to
> > > get setup the pci_host_bridge pointer for the remaining architectures that
> > > still implement it.  I did look for any easy way but couldn't find one.
> > > But then I don't really know this area of the PCI code too well.
> >
> > If the platform doesn't already have a pointer to its pci_host_bridge
> > structure, it can call pci_find_host_bridge(bus->dev) to get it.
> >
> > Ideally, platforms (either arch specific code or drivers/pci/controller)
> > should call pci_alloc_host_bridge()/pci_register_host_bridge()
> > instead of the traditional pci_create_root_bus() that is less flexible.
> > That way, the driver specific structure can get allocated together with
> > the host bridge.
>
> Yes that's basically the gist, it should be a matter of identifying code
> that creates the root bus (explicitly or implicitly - ie
> pci_scan_root_bus()) and patching it up, probably something to be done
> (and moved into -next) early -rc* since it is something that can easily
> break the world if we miss some code paths. I will have a look into
> that and can take this patch for that series if it does not go into
> v4.19.

I've started prototyping something now, will post something tomorrow.
I have an idea for a version that should be minimally invasive, at the
cost of duplicating some code in drivers that still use the old interfaces.

        Arnd

[PATCH 1/3] PCI: add a callback to struct pci_host_bridge for adding a new device

[Arnd Bergmann]

On Mon, Aug 6, 2018 at 4:56 PM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Mon, Aug 06, 2018 at 03:54:13PM +0200, Arnd Bergmann wrote:
> > On Mon, Aug 6, 2018 at 2:25 PM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > On Mon, Aug 06, 2018 at 12:23:10PM +0100, Lorenzo Pieralisi wrote:
> > > > >     pci_configure_device(dev);
> > > > > @@ -2328,6 +2329,11 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> > > > >     ret = pcibios_add_device(dev);
> > > > >     WARN_ON(ret < 0);
> > > > >
> > > > > +   if (host->add_device) {
> > > > > +           ret = host->add_device(dev);
> > > > > +           WARN_ON(ret < 0);
> > > > > +   }
> > > >
> > > > This looks fine; we could go a step further and make the hunk above
> > > > the default (weak) implementation of pcibios_add_device() that is
> > > > currently a NOP returning 0, I will remove it for v4.20.
> > >
> > > I'd love to see pcibios_add_device go away entirely.  But I wonder how to
> > > get setup the pci_host_bridge pointer for the remaining architectures that
> > > still implement it.  I did look for any easy way but couldn't find one.
> > > But then I don't really know this area of the PCI code too well.
> >
> > If the platform doesn't already have a pointer to its pci_host_bridge
> > structure, it can call pci_find_host_bridge(bus->dev) to get it.
> >
> > Ideally, platforms (either arch specific code or drivers/pci/controller)
> > should call pci_alloc_host_bridge()/pci_register_host_bridge()
> > instead of the traditional pci_create_root_bus() that is less flexible.
> > That way, the driver specific structure can get allocated together with
> > the host bridge.
>
> Yes that's basically the gist, it should be a matter of identifying code
> that creates the root bus (explicitly or implicitly - ie
> pci_scan_root_bus()) and patching it up, probably something to be done
> (and moved into -next) early -rc* since it is something that can easily
> break the world if we miss some code paths. I will have a look into
> that and can take this patch for that series if it does not go into
> v4.19.

I've started prototyping something now, will post something tomorrow.
I have an idea for a version that should be minimally invasive, at the
cost of duplicating some code in drivers that still use the old interfaces.

        Arnd

Re: [PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

On Thu, Aug 02, 2018 at 05:56:23PM +0100, Lorenzo Pieralisi wrote:
> I could have taken it for this merge window but unfortunately it is a
> bit late (ie PCI patches queue is close), I can certainly queue it up
> for v4.20 though.

It is a little sad that we can't take a trivial kconfig patch more than a
week before the end of the merge window.  But then against given how long
a version of this patch has been around and not submitted who am I to
complain..

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

On Thu, Aug 02, 2018 at 05:56:23PM +0100, Lorenzo Pieralisi wrote:
> I could have taken it for this merge window but unfortunately it is a
> bit late (ie PCI patches queue is close), I can certainly queue it up
> for v4.20 though.

It is a little sad that we can't take a trivial kconfig patch more than a
week before the end of the merge window.  But then against given how long
a version of this patch has been around and not submitted who am I to
complain..

Re: [PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Lorenzo Pieralisi]

On Wed, Aug 01, 2018 at 05:14:03PM +0200, Christoph Hellwig wrote:
> There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
> architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
> and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
> port will need to be able to enable PCIE_XILINX in order to have PCIe
> support.
> 
> This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
> depends on OF, which I believe is the only true dependency.
> 
> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> [hch: switch to OF instead of OF_PCI now that the latter is gone]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/controller/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I could have taken it for this merge window but unfortunately it is a
bit late (ie PCI patches queue is close), I can certainly queue it up
for v4.20 though.

Lorenzo

> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index cc9fa02d32a0..0f7ce5eaeac8 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -102,7 +102,7 @@ config PCI_HOST_GENERIC
>  
>  config PCIE_XILINX
>  	bool "Xilinx AXI PCIe host bridge support"
> -	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
> +	depends on OF || COMPILE_TEST
>  	help
>  	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
>  	  Host Bridge driver.
> -- 
> 2.18.0
> 

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Lorenzo Pieralisi]

On Wed, Aug 01, 2018 at 05:14:03PM +0200, Christoph Hellwig wrote:
> There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
> architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
> and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
> port will need to be able to enable PCIE_XILINX in order to have PCIe
> support.
> 
> This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
> depends on OF, which I believe is the only true dependency.
> 
> Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
> [hch: switch to OF instead of OF_PCI now that the latter is gone]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/controller/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I could have taken it for this merge window but unfortunately it is a
bit late (ie PCI patches queue is close), I can certainly queue it up
for v4.20 though.

Lorenzo

> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index cc9fa02d32a0..0f7ce5eaeac8 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -102,7 +102,7 @@ config PCI_HOST_GENERIC
>  
>  config PCIE_XILINX
>  	bool "Xilinx AXI PCIe host bridge support"
> -	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
> +	depends on OF || COMPILE_TEST
>  	help
>  	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
>  	  Host Bridge driver.
> -- 
> 2.18.0
> 

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
port will need to be able to enable PCIE_XILINX in order to have PCIe
support.

This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
depends on OF, which I believe is the only true dependency.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: switch to OF instead of OF_PCI now that the latter is gone]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index cc9fa02d32a0..0f7ce5eaeac8 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -102,7 +102,7 @@ config PCI_HOST_GENERIC
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
+	depends on OF || COMPILE_TEST
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.18.0

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
port will need to be able to enable PCIE_XILINX in order to have PCIe
support.

This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
depends on OF, which I believe is the only true dependency.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: switch to OF instead of OF_PCI now that the latter is gone]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index cc9fa02d32a0..0f7ce5eaeac8 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -102,7 +102,7 @@ config PCI_HOST_GENERIC
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
+	depends on OF || COMPILE_TEST
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.18.0

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

From: Palmer Dabbelt <palmer@dabbelt.com>

There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
port will need to be able to enable PCIE_XILINX in order to have PCIe
support.

This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
depends on OF, which I believe is the only true dependency.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: replaced the now removed OF_PCI dependency with just OF]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 18fa09b3ac8f..1fc3d4eb373d 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -103,7 +103,7 @@ config PCI_HOST_GENERIC
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
+	depends on OF || COMPILE_TEST
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.17.1

[PATCH 3/3] PCI/xilinx: Depend on OF instead of the ARCH

[Christoph Hellwig]

From: Palmer Dabbelt <palmer@dabbelt.com>

There isn't a hard dependency of the Xilinx AXI-PCIe host bridge on any
architecture.  For example: at SiFive we map RISC-V cores to Xilinx FPGAs
and connect the Xilinx IP via a TileLink adapter, so the RISC-V Linux
port will need to be able to enable PCIE_XILINX in order to have PCIe
support.

This patch decouples the PCIE_XILINX support from ARCH.  Instead it just
depends on OF, which I believe is the only true dependency.

Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
[hch: replaced the now removed OF_PCI dependency with just OF]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/controller/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 18fa09b3ac8f..1fc3d4eb373d 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -103,7 +103,7 @@ config PCI_HOST_GENERIC
 
 config PCIE_XILINX
 	bool "Xilinx AXI PCIe host bridge support"
-	depends on ARCH_ZYNQ || MICROBLAZE || (MIPS && PCI_DRIVERS_GENERIC) || COMPILE_TEST
+	depends on OF || COMPILE_TEST
 	help
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
-- 
2.17.1