[U-Boot] [PATCH 0/3] Introduce APIs for multi PCIe host controller platforms

[U-Boot] [PATCH 0/3] Introduce APIs for multi PCIe host controller platforms

[Vladimir Olovyannikov]

This patch set contains proposed API changes to the pci uclass for
multi PCIe host controller platforms.

1. Add changes to fix bus number of a host bridge

   Differentiate bus numbers hierarchy for root bridges.
   All bus numbers are device sequence numbers minus
   root bridge sequence number.
   This way, every root bridge and its downstream EPs bus
   numbers start from 0.
   Thus, root bridges are different hierarchy of bus numbers.

2. Get next device fail with driver probe fail

   In Multi PCIe host controller platforms, if one PCIe host driver
   probe fails for any reason, the next PCIe host controller device
   pointer should be tried with its driver probe. Instead, currently
   the code simply stops enumeration. Add the feature described above.

3. Add ability to parse dma-regions given in PCIe host controller's
   DT node.


Srinath Mannam (3):
  drivers: pci: Fix Host bridge bus number issue
  drivers: core: uclass: Get next device fail with driver probe fail
  drivers: pci: pci-uclass: Get PCI dma regions support

 drivers/core/uclass.c    |  2 +-
 drivers/pci/pci-uclass.c | 49 +++++++++++++++++++++++++++++++++++++---
 drivers/pci/pci_auto.c   |  6 ++++-
 include/pci.h            |  2 ++
 4 files changed, 54 insertions(+), 5 deletions(-)

-- 
2.17.1

[U-Boot] [PATCH 1/3] drivers: pci: Fix Host bridge bus number issue

[Vladimir Olovyannikov]

From: Srinath Mannam <srinath.mannam@broadcom.com>

Add changes to fix bus number of host bridge is set with device
sequence number issue.
All devices are managed using device sequence number. For PCIe,
devices enabled in DTS are added under PCIE_CLASS with consecutive
device sequence numbers to scan all pcie devices in sequence using
device sequence number. If a device is a bridge then it will enumerate
all endpoints in that bridge, and give sequence numbers in that order.
However, the parent device is a root bridge.
The solution is all bus numbers are device sequence number minus
root bridge sequence number. This way, every root bridge and its
downstream EPs bus numbers start from 0.
So root bridges are different hierarchy of bus numbers.

Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 drivers/pci/pci-uclass.c | 8 +++++---
 drivers/pci/pci_auto.c   | 6 +++++-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 896cb6b23a..eb7a01fd55 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -47,8 +47,9 @@ pci_dev_t dm_pci_get_bdf(struct udevice *dev)
 {
 	struct pci_child_platdata *pplat = dev_get_parent_platdata(dev);
 	struct udevice *bus = dev->parent;
+	struct udevice *ctrl = pci_get_controller(dev);
 
-	return PCI_ADD_BUS(bus->seq, pplat->devfn);
+	return PCI_ADD_BUS(bus->seq - ctrl->seq, pplat->devfn);
 }
 
 /**
@@ -760,11 +761,12 @@ int pci_bind_bus_devices(struct udevice *bus)
 	pci_dev_t bdf, end;
 	bool found_multi;
 	int ret;
+	struct udevice *ctrl = pci_get_controller(bus);
 
 	found_multi = false;
-	end = PCI_BDF(bus->seq, PCI_MAX_PCI_DEVICES - 1,
+	end = PCI_BDF(bus->seq - ctrl->seq, PCI_MAX_PCI_DEVICES - 1,
 		      PCI_MAX_PCI_FUNCTIONS - 1);
-	for (bdf = PCI_BDF(bus->seq, 0, 0); bdf <= end;
+	for (bdf = PCI_BDF(bus->seq - ctrl->seq, 0, 0); bdf <= end;
 	     bdf += PCI_BDF(0, 0, 1)) {
 		struct pci_child_platdata *pplat;
 		struct udevice *dev;
diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 28667bde8d..42bf51fef2 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -176,8 +176,12 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
 	struct pci_region *pci_io;
 	u16 cmdstat, prefechable_64;
 	struct udevice *ctlr = pci_get_controller(dev);
+	struct udevice *parent = dev->parent;
 	struct pci_controller *ctlr_hose = dev_get_uclass_priv(ctlr);
 
+	if (!parent)
+		return;
+
 	pci_mem = ctlr_hose->pci_mem;
 	pci_prefetch = ctlr_hose->pci_prefetch;
 	pci_io = ctlr_hose->pci_io;
@@ -188,7 +192,7 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
 
 	/* Configure bus number registers */
 	dm_pci_write_config8(dev, PCI_PRIMARY_BUS,
-			     PCI_BUS(dm_pci_get_bdf(dev)) - ctlr->seq);
+			     parent->seq - ctlr->seq);
 	dm_pci_write_config8(dev, PCI_SECONDARY_BUS, sub_bus - ctlr->seq);
 	dm_pci_write_config8(dev, PCI_SUBORDINATE_BUS, 0xff);
 
-- 
2.17.1

[U-Boot] [PATCH 1/3] drivers: pci: Fix Host bridge bus number issue

[Bin Meng]

Hi Vladimir,

On Sat, Nov 23, 2019 at 6:23 AM Vladimir Olovyannikov
<vladimir.olovyannikov@broadcom.com> wrote:
>
> From: Srinath Mannam <srinath.mannam@broadcom.com>
>
> Add changes to fix bus number of host bridge is set with device
> sequence number issue.
> All devices are managed using device sequence number. For PCIe,
> devices enabled in DTS are added under PCIE_CLASS with consecutive
> device sequence numbers to scan all pcie devices in sequence using
> device sequence number. If a device is a bridge then it will enumerate
> all endpoints in that bridge, and give sequence numbers in that order.
> However, the parent device is a root bridge.
> The solution is all bus numbers are device sequence number minus
> root bridge sequence number. This way, every root bridge and its
> downstream EPs bus numbers start from 0.
> So root bridges are different hierarchy of bus numbers.

Could you please give an example for better understanding?

>
> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> ---
>  drivers/pci/pci-uclass.c | 8 +++++---
>  drivers/pci/pci_auto.c   | 6 +++++-
>  2 files changed, 10 insertions(+), 4 deletions(-)
>

Regards,
Bin

[U-Boot] [PATCH 2/3] drivers: core: uclass: Get next device fail with driver probe fail

[Vladimir Olovyannikov]

From: Srinath Mannam <srinath.mannam@broadcom.com>

Add changes to fix get next device failed if driver probe failed
issue. In Multi PCIe host controller platforms, if one PCIe host
driver probe failed with any reason then it stops to find next
PCIe host controller device pointer to call its driver probe.

Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 drivers/core/uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index c520ef113a..ab50f8f6db 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -442,7 +442,7 @@ int uclass_get_device_tail(struct udevice *dev, int ret, struct udevice **devp)
 	assert(dev);
 	ret = device_probe(dev);
 	if (ret)
-		return ret;
+		dev_dbg(dev, "%s device_probe failed\n", __func__);
 
 	*devp = dev;
 
-- 
2.17.1

[U-Boot] [PATCH 2/3] drivers: core: uclass: Get next device fail with driver probe fail

[Bin Meng]

Hi Vladimir,

On Sat, Nov 23, 2019 at 6:23 AM Vladimir Olovyannikov
<vladimir.olovyannikov@broadcom.com> wrote:
>
> From: Srinath Mannam <srinath.mannam@broadcom.com>
>
> Add changes to fix get next device failed if driver probe failed
> issue. In Multi PCIe host controller platforms, if one PCIe host
> driver probe failed with any reason then it stops to find next
> PCIe host controller device pointer to call its driver probe.
>
> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> ---
>  drivers/core/uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
> index c520ef113a..ab50f8f6db 100644
> --- a/drivers/core/uclass.c
> +++ b/drivers/core/uclass.c
> @@ -442,7 +442,7 @@ int uclass_get_device_tail(struct udevice *dev, int ret, struct udevice **devp)
>         assert(dev);
>         ret = device_probe(dev);
>         if (ret)
> -               return ret;
> +               dev_dbg(dev, "%s device_probe failed\n", __func__);

Could you please try pci_init() which I believe should satisfy your requirement?

>
>         *devp = dev;
>

Regards,
Bin

[U-Boot] [PATCH 3/3] drivers: pci: pci-uclass: Get PCI dma regions support

[Vladimir Olovyannikov]

From: Srinath Mannam <srinath.mannam@broadcom.com>

Add API to parse dma-regions given in PCIe host controller
DT node.

Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 drivers/pci/pci-uclass.c | 41 ++++++++++++++++++++++++++++++++++++++++
 include/pci.h            |  2 ++
 2 files changed, 43 insertions(+)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index eb7a01fd55..ddc2d5cf2c 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -1166,6 +1166,47 @@ ulong pci_conv_size_to_32(ulong old, ulong value, uint offset,
 	return value;
 }
 
+int pci_get_dma_regions(struct udevice *dev, struct pci_region *memp, int index)
+{
+	int pci_addr_cells, addr_cells, size_cells;
+	int cells_per_record;
+	const u32 *prop;
+	int len;
+	int i = 0;
+
+	prop = ofnode_get_property(dev_ofnode(dev), "dma-ranges", &len);
+	if (!prop) {
+		dev_err(dev, "%s: Cannot decode dma-ranges\n", __func__);
+		return -EINVAL;
+	}
+
+	pci_addr_cells = ofnode_read_simple_addr_cells(dev_ofnode(dev));
+	addr_cells = ofnode_read_simple_addr_cells(dev_ofnode(dev->parent));
+	size_cells = ofnode_read_simple_size_cells(dev_ofnode(dev));
+
+	/* PCI addresses are always 3-cells */
+	len /= sizeof(u32);
+	cells_per_record = pci_addr_cells + addr_cells + size_cells;
+	debug("%s: len=%d, cells_per_record=%d\n", __func__, len,
+	      cells_per_record);
+
+	while (len) {
+		memp->bus_start = fdtdec_get_number(prop + 1, 2);
+		prop += pci_addr_cells;
+		memp->phys_start = fdtdec_get_number(prop, addr_cells);
+		prop += addr_cells;
+		memp->size = fdtdec_get_number(prop, size_cells);
+		prop += size_cells;
+
+		if (i == index)
+			return 0;
+		i++;
+		len -= cells_per_record;
+	}
+
+	return -EINVAL;
+}
+
 int pci_get_regions(struct udevice *dev, struct pci_region **iop,
 		    struct pci_region **memp, struct pci_region **prefp)
 {
diff --git a/include/pci.h b/include/pci.h
index ff59ac0e69..ef55f54ea5 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -1284,6 +1284,8 @@ struct udevice *pci_get_controller(struct udevice *dev);
 int pci_get_regions(struct udevice *dev, struct pci_region **iop,
 		    struct pci_region **memp, struct pci_region **prefp);
 
+int
+pci_get_dma_regions(struct udevice *dev, struct pci_region *memp, int index);
 /**
  * dm_pci_write_bar32() - Write the address of a BAR
  *
-- 
2.17.1

[U-Boot] [PATCH 3/3] drivers: pci: pci-uclass: Get PCI dma regions support

[Bin Meng]

Hi Vladimir,

On Sat, Nov 23, 2019 at 6:23 AM Vladimir Olovyannikov
<vladimir.olovyannikov@broadcom.com> wrote:
>
> From: Srinath Mannam <srinath.mannam@broadcom.com>
>
> Add API to parse dma-regions given in PCIe host controller
> DT node.
>
> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> ---
>  drivers/pci/pci-uclass.c | 41 ++++++++++++++++++++++++++++++++++++++++
>  include/pci.h            |  2 ++
>  2 files changed, 43 insertions(+)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index eb7a01fd55..ddc2d5cf2c 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -1166,6 +1166,47 @@ ulong pci_conv_size_to_32(ulong old, ulong value, uint offset,
>         return value;
>  }
>
> +int pci_get_dma_regions(struct udevice *dev, struct pci_region *memp, int index)
> +{
> +       int pci_addr_cells, addr_cells, size_cells;
> +       int cells_per_record;
> +       const u32 *prop;
> +       int len;
> +       int i = 0;
> +
> +       prop = ofnode_get_property(dev_ofnode(dev), "dma-ranges", &len);
> +       if (!prop) {
> +               dev_err(dev, "%s: Cannot decode dma-ranges\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       pci_addr_cells = ofnode_read_simple_addr_cells(dev_ofnode(dev));
> +       addr_cells = ofnode_read_simple_addr_cells(dev_ofnode(dev->parent));
> +       size_cells = ofnode_read_simple_size_cells(dev_ofnode(dev));
> +
> +       /* PCI addresses are always 3-cells */
> +       len /= sizeof(u32);
> +       cells_per_record = pci_addr_cells + addr_cells + size_cells;
> +       debug("%s: len=%d, cells_per_record=%d\n", __func__, len,
> +             cells_per_record);
> +
> +       while (len) {
> +               memp->bus_start = fdtdec_get_number(prop + 1, 2);
> +               prop += pci_addr_cells;
> +               memp->phys_start = fdtdec_get_number(prop, addr_cells);
> +               prop += addr_cells;
> +               memp->size = fdtdec_get_number(prop, size_cells);
> +               prop += size_cells;
> +
> +               if (i == index)
> +                       return 0;
> +               i++;
> +               len -= cells_per_record;
> +       }
> +
> +       return -EINVAL;
> +}
> +
>  int pci_get_regions(struct udevice *dev, struct pci_region **iop,
>                     struct pci_region **memp, struct pci_region **prefp)
>  {
> diff --git a/include/pci.h b/include/pci.h
> index ff59ac0e69..ef55f54ea5 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -1284,6 +1284,8 @@ struct udevice *pci_get_controller(struct udevice *dev);
>  int pci_get_regions(struct udevice *dev, struct pci_region **iop,
>                     struct pci_region **memp, struct pci_region **prefp);
>
> +int

nits: this should be put on the same line of the function name.
Besides, would you please add the function descriptions with
parameters and return value?

> +pci_get_dma_regions(struct udevice *dev, struct pci_region *memp, int index);

Should have one blank line

>  /**
>   * dm_pci_write_bar32() - Write the address of a BAR
>   *
> --

Regards,
Bin