On Wed, Aug 19, 2015 at 3:11 AM, Dinh Nguyen [off-list ref] wrote:

On Mon, Aug 17, 2015 at 4:09 AM, Ley Foon Tan [off-list ref] wrote:

quoted

This patch adds the Altera PCIe host controller driver.

Signed-off-by: Ley Foon Tan <redacted>
---
 drivers/pci/host/Kconfig       |   7 +
 drivers/pci/host/Makefile      |   1 +
 drivers/pci/host/pcie-altera.c | 543 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 551 insertions(+)
 create mode 100644 drivers/pci/host/pcie-altera.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 675c2d1..4b4754a 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig

@@ -145,4 +145,11 @@ config PCIE_IPROC_BCMA
          Say Y here if you want to use the Broadcom iProc PCIe controller
          through the BCMA bus interface

<snip>

quoted

+
+/* Address translation table entry size */
+#define ATT_ENTRY_SIZE         8
+
+#define DWORD_MASK             3
+
+struct altera_pcie {
+       struct platform_device  *pdev;
+       struct resource         *txs;

You have "Txs" documented in the bindings document, you have a pointer
here, but you've never used it
anywhre in the code? What is it for?

Good catch. Forgot to remove this txs field here, we no longer require
to keep this in struct.

quoted

+       void __iomem            *cra_base;
+       int                     irq;
+       u8                      root_bus_nr;
+       struct irq_domain               *irq_domain;
+       struct resource         bus_range;
+       struct list_head                resources;
+};
+

<snip>

quoted

+
+static int tlp_cfg_dword_read(struct altera_pcie *pcie, u8 bus, u32 devfn,
+                             int where, u32 *value)
+{
+       int ret;
+       u32 headers[TLP_HDR_SIZE];
+
+       if (bus == pcie->root_bus_nr)
+               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD0);
+       else
+               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGRD1);
+
+       headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
+                                       TLP_READ_TAG);
+       headers[2] = TLP_CFG_DW2(bus, devfn, where);
+
+       tlp_write_packet(pcie, headers, 0);
+
+       ret = tlp_read_packet(pcie, value);
+       if (ret)
+               *value = ~0UL;  /* return 0xFFFFFFFF if error */
+
+       return ret;
+}
+
+static int tlp_cfg_dword_write(struct altera_pcie *pcie, u8 bus, u32 devfn,
+                              int where, u32 value)
+{
+       u32 headers[TLP_HDR_SIZE];
+
+       if (bus == pcie->root_bus_nr)
+               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR0);
+       else
+               headers[0] = TLP_CFG_DW0(TLP_FMTTYPE_CFGWR1);
+
+       headers[1] = TLP_CFG_DW1(TLP_REQ_ID(pcie->root_bus_nr, devfn),
+                                       TLP_WRITE_TAG);
+       headers[2] = TLP_CFG_DW2(bus, devfn, where);
+
+       tlp_write_packet(pcie, headers, value);
+
+       tlp_read_packet(pcie, NULL);

You need to check for the error here.

Okay.

quoted

+
+       /* Keep an eye out for changes to the root bus number */
+       if ((bus == pcie->root_bus_nr) && (where == PCI_PRIMARY_BUS))
+               pcie->root_bus_nr = (u8)(value);
+
+       return PCIBIOS_SUCCESSFUL;
+}
+

<snip>

quoted

+
+static int altera_pcie_parse_dt(struct altera_pcie *pcie)
+{
+       struct resource *cra;
+       struct platform_device *pdev = pcie->pdev;
+
+       cra = platform_get_resource_byname(pdev, IORESOURCE_MEM, "Cra");
+       if (!cra) {
+               cra = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cra");
+               if (!cra) {
+                       dev_err(&pdev->dev,
+                               "no cra memory resource defined\n");
+                       return -ENODEV;
+               }
+       }
+

What about "Txs"?

We doesn't need to get resource for "Txs" here. We use standard pci
binding with "ranges" dts parameter to map the pci memory region.
Our dts generator still will generate this "Txs" dts parameter,
because it is one of Avalon slave port of PCIe IP.


Regards
Ley Foon