PCI: Add Intel PCIe Driver and respective dt-binding yaml file

Add support to PCIe RC controller on Intel Gateway SoCs. PCIe controller is based of Synopsys DesignWare PCIe core.

Intel PCIe driver requires Upconfigure support, Fast Training Sequence and link speed configurations. So adding the respective helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed configuration so defining it in pci_regs.h.

Also, mark Intel PCIe driver depends on MSI IRQ Domain as Synopsys DesignWare framework depends on the PCI_MSI_IRQ_DOMAIN.

Signed-off-by Dilip Kota <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

Reviewed-by Andrew Murray <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Acked-by Gustavo Pimentel <[email protected]>

drivers/pci/controller/dwc/pcie-designware.c +14 -0

+struct intel_pcie_port {
+	struct dw_pcie		pci;
+	void __iomem		*app_base;
+	struct gpio_desc	*reset_gpio;
+	u32			rst_intrvl;
+	u32			max_speed;
+	u32			link_gen;
+	u32			max_width;
+	u32			n_fts;
+	struct clk		*core_clk;
+	struct reset_control	*core_rst;
+	struct phy		*phy;
+	u8			pcie_cap_ofst;
+};

+int intel_pcie_msi_init(struct pcie_port *pp)

+}

Quoted 22 lines · Dilip Kota

Add support to PCIe RC controller on Intel Gateway SoCs. PCIe controller is based of Synopsys DesignWare PCIe core.

Intel PCIe driver requires Upconfigure support, Fast Training Sequence and link speed configurations. So adding the respective helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed configuration so defining it in pci_regs.h.

Also, mark Intel PCIe driver depends on MSI IRQ Domain as Synopsys DesignWare framework depends on the PCI_MSI_IRQ_DOMAIN.

Signed-off-by Dilip Kota <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

Reviewed-by Andrew Murray <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Acked-by Gustavo Pimentel <[email protected]>

drivers/pci/controller/dwc/pcie-designware.c +0 -0

@@ -14,6 +14,8 @@
 

drivers/pci/controller/dwc/pcie-designware.c line 17

Quoted 3 lines

drivers/pci/controller/dwc/pcie-designware.c +1 -0

  #include "pcie-designware.h"
 
+extern const unsigned char pcie_link_speed[];

This shouldn't be needed; there's a declaration in drivers/pci/pci.h.

drivers/pci/controller/dwc/pcie-designware.c

Quoted 6 lines

drivers/pci/controller/dwc/pcie-designware.c +5 -0

+struct intel_pcie_soc {
+	unsigned int pcie_ver;
+	unsigned int pcie_atu_offset;
+	u32 num_viewport;
+};

Looks a little strange to have the fields below lined up but the ones above not.

Quoted 20 lines

drivers/pci/controller/dwc/pcie-designware.c +19 -0

+struct intel_pcie_port {
+	struct dw_pcie		pci;
+	void __iomem		*app_base;
+	struct gpio_desc	*reset_gpio;
+	u32			rst_intrvl;
+	u32			max_speed;
+	u32			link_gen;
+	u32			max_width;
+	u32			n_fts;
+	struct clk		*core_clk;
+	struct reset_control	*core_rst;
+	struct phy		*phy;
+	u8			pcie_cap_ofst;
+};
+
+static void pcie_update_bits(void __iomem *base, u32 ofs, u32 mask, u32 val)
+{
+	u32 old;
+

drivers/pci/controller/dwc/pcie-designware.c

Quoted 5 lines

drivers/pci/controller/dwc/pcie-designware.c +5 -0

+	old = readl(base + ofs);
+	val = (old & ~mask) | (val & mask);
+
+	if (val != old)
+		writel(val, base + ofs);

I assume this is never used on registers where the "old & ~mask" part contains RW1C bits? If there are RW1C bits in that part, this will corrupt them.

drivers/pci/controller/dwc/pcie-designware.c

Quoted 6 lines

drivers/pci/controller/dwc/pcie-designware.c +5 -0

+	if (!lpp->pcie_cap_ofst) {
+		ret = dw_pcie_find_capability(&lpp->pci, PCI_CAP_ID_EXP);
+		if (!ret) {
+			ret = -ENXIO;
+			dev_err(dev, "Invalid PCIe capability offset\n");

Some of your messages start with a capital letter, others not.

Quoted 2 lines

+int intel_pcie_msi_init(struct pcie_port *pp)

You might add a comment here like the one at ks_pcie_am654_msi_host_init(). Since the users of the .msi_host_init() function pointer only call the function if the pointer is non-NULL, it's not completely obvious why you have this stub function.

drivers/pci/controller/dwc/pcie-designware.c

Quoted 8 lines

drivers/pci/controller/dwc/pcie-designware.c +7 -0

+{
+	/* PCIe MSI/MSIx is handled by MSI in x86 processor */
+	return 0;
+}
+	/*
+	 * Intel PCIe doesn't configure IO region, so set viewport
+	 * to not to perform IO region access.

s/to not to/to not/

drivers/pci/controller/dwc/pcie-designware.c

Quoted 5 lines

drivers/pci/controller/dwc/pcie-designware.c +4 -0

+	 */
+	pci->num_viewport = data->num_viewport;
+
+	dev_info(dev, "Intel PCIe Root Complex Port init done\n");

Probably superfluous.

drivers/pci/controller/dwc/pcie-designware.c

Quoted 3 lines

drivers/pci/controller/dwc/pcie-designware.c +2 -0

+
+	return ret;

Since the return value is known here:

   return 0;

Quoted 2 lines

+}

Quoted 20 lines · Bjorn Helgaas

Quoted 18 lines · Dilip Kota

Add support to PCIe RC controller on Intel Gateway SoCs. PCIe controller is based of Synopsys DesignWare PCIe core.

Intel PCIe driver requires Upconfigure support, Fast Training Sequence and link speed configurations. So adding the respective helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed configuration so defining it in pci_regs.h.

Also, mark Intel PCIe driver depends on MSI IRQ Domain as Synopsys DesignWare framework depends on the PCI_MSI_IRQ_DOMAIN.

Signed-off-by Dilip Kota <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

Reviewed-by Andrew Murray <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Acked-by Gustavo Pimentel <[email protected]>

Quoted 12 lines

drivers/pci/controller/dwc/pcie-intel-gw.c line 88

Quoted 9 lines

drivers/pci/controller/dwc/pcie-intel-gw.c new file +9 -0

+static void pcie_update_bits(void __iomem *base, u32 ofs, u32 mask, u32 val)
+{
+	u32 old;
+
+	old = readl(base + ofs);
+	val = (old & ~mask) | (val & mask);
+
+	if (val != old)
+		writel(val, base + ofs);

I assume this is never used on registers where the "old & ~mask" part contains RW1C bits? If there are RW1C bits in that part, this will corrupt them.

There is no impact because RW1C bits of respective registers are 0s at the time of this function call.

I see, this patch series is merged in the maintainer tree. Should i need to submit as a separate patch on top of maintainer tree or submit the new version of whole patch series?
Please let me know the best practice.

Quoted 34 lines · Dilip Kota

Quoted 20 lines · Bjorn Helgaas

Quoted 18 lines · Dilip Kota

Add support to PCIe RC controller on Intel Gateway SoCs. PCIe controller is based of Synopsys DesignWare PCIe core.

Intel PCIe driver requires Upconfigure support, Fast Training Sequence and link speed configurations. So adding the respective helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed configuration so defining it in pci_regs.h.

Also, mark Intel PCIe driver depends on MSI IRQ Domain as Synopsys DesignWare framework depends on the PCI_MSI_IRQ_DOMAIN.

Signed-off-by Dilip Kota <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

Reviewed-by Andrew Murray <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Acked-by Gustavo Pimentel <[email protected]>

Quoted 12 lines

drivers/pci/controller/dwc/pcie-intel-gw.c line 88

Quoted 9 lines

drivers/pci/controller/dwc/pcie-intel-gw.c new file +9 -0

+static void pcie_update_bits(void __iomem *base, u32 ofs, u32 mask, u32 val)
+{
+	u32 old;
+
+	old = readl(base + ofs);
+	val = (old & ~mask) | (val & mask);
+
+	if (val != old)
+		writel(val, base + ofs);

I assume this is never used on registers where the "old & ~mask" part contains RW1C bits? If there are RW1C bits in that part, this will corrupt them.

Quoted 2 lines

There is no impact because RW1C bits of respective registers are 0s at the time of this function call.

Sounds ... dangerous, but I'll take your word for it.

Quoted 4 lines

I see, this patch series is merged in the maintainer tree. Should i need to submit as a separate patch on top of maintainer tree or submit the new version of whole patch series?
Please let me know the best practice.

Sorry, I didn't realize this had already been merged to Lorenzo's tree. But it's not upstream (in Linus' tree) yet. I don't know how Andrew and Lorenzo want to handle this. None of these are important, so you could just ignore these comments.

What I personally would do is rebase the branch, e.g., lpieralisi/pci/dwc, and apply an incremental patch. But it's up to Andrew and Lorenzo whether they want to do anything.

Bjorn

Intel PCIe is Synopsys based controller. Intel PCIe driver uses DesignWare PCIe framework for host initialization and register configurations.

Changes on v10:
Rebase the patches on mainline v5.4

Squashed the patch that fixes the below issue to this patch series.

WARNING: unmet direct dependencies detected for PCIE_DW_HOST
Depends on [n]: PCI [=y] && PCI_MSI_IRQ_DOMAIN [=n]
Selected by [y]:
- PCIE_INTEL_GW [=y] && PCI [=y] && OF [=y] && (X86 [=y] || COMPILE_TEST [=n])
"reportedby Randy Dunlap <[email protected]>"

Dilip Kota (3):
dt-bindings: PCI: intel: Add YAML schemas for the PCIe RC controller
PCI: dwc: intel: PCIe RC controller driver
PCI: artpec6: Configure FTS with dwc helper function

 .../devicetree/bindings/pci/intel-gw-pcie.yaml     | 138 ++++++
 drivers/pci/controller/dwc/Kconfig                 |  11 +
 drivers/pci/controller/dwc/Makefile                |   1 +
 drivers/pci/controller/dwc/pcie-artpec6.c          |   8 +-
 drivers/pci/controller/dwc/pcie-designware.c       |  57 +++
 drivers/pci/controller/dwc/pcie-designware.h       |  12 +
 drivers/pci/controller/dwc/pcie-intel-gw.c         | 545 +++++++++++++++++++++
 include/uapi/linux/pci_regs.h                      |   1 +
 8 files changed, 766 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c

Signature · 1 line

2.11.0