2018-10-25 11:15:05

by Bao Xiaowei

[permalink] [raw]
Subject: [PATCH 1/6] arm64: dts: Add the status property disable PCIe

From: Bao Xiaowei <[email protected]>

Add the status property disable the PCIe, the property will be enable
by bootloader.

Signed-off-by: Bao Xiaowei <[email protected]>
---
arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 1 +
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 3 +++
arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 3 +++
arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 3 +++
arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ++++
5 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
index 5da732f..21f2b3b 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -496,6 +496,7 @@
<0000 0 0 2 &gic 0 111 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic 0 112 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic 0 113 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
};
};

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 3fed504..760d510 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -683,6 +683,7 @@
<0000 0 0 2 &gic 0 111 0x4>,
<0000 0 0 3 &gic 0 112 0x4>,
<0000 0 0 4 &gic 0 113 0x4>;
+ status = "disabled";
};

pcie@3500000 {
@@ -708,6 +709,7 @@
<0000 0 0 2 &gic 0 121 0x4>,
<0000 0 0 3 &gic 0 122 0x4>,
<0000 0 0 4 &gic 0 123 0x4>;
+ status = "disabled";
};

pcie@3600000 {
@@ -733,6 +735,7 @@
<0000 0 0 2 &gic 0 155 0x4>,
<0000 0 0 3 &gic 0 156 0x4>,
<0000 0 0 4 &gic 0 157 0x4>;
+ status = "disabled";
};
};

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 51cbd50..64d334c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -652,6 +652,7 @@
<0000 0 0 2 &gic GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
};

pcie@3500000 {
@@ -677,6 +678,7 @@
<0000 0 0 2 &gic GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
};

pcie@3600000 {
@@ -702,6 +704,7 @@
<0000 0 0 2 &gic GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
};

};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index a07f612..9deb9cb 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -533,6 +533,7 @@
<0000 0 0 2 &gic 0 0 0 110 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic 0 0 0 111 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic 0 0 0 112 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
};

pcie@3500000 {
@@ -557,6 +558,7 @@
<0000 0 0 2 &gic 0 0 0 115 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic 0 0 0 116 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic 0 0 0 117 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
};

pcie@3600000 {
@@ -581,6 +583,7 @@
<0000 0 0 2 &gic 0 0 0 120 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic 0 0 0 121 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic 0 0 0 122 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
};

cluster1_core0_watchdog: wdt@c000000 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index d188774..5732e3b 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -648,6 +648,7 @@
<0000 0 0 2 &gic 0 0 0 110 4>,
<0000 0 0 3 &gic 0 0 0 111 4>,
<0000 0 0 4 &gic 0 0 0 112 4>;
+ status = "disabled";
};

pcie2: pcie@3500000 {
@@ -669,6 +670,7 @@
<0000 0 0 2 &gic 0 0 0 115 4>,
<0000 0 0 3 &gic 0 0 0 116 4>,
<0000 0 0 4 &gic 0 0 0 117 4>;
+ status = "disabled";
};

pcie3: pcie@3600000 {
@@ -690,6 +692,7 @@
<0000 0 0 2 &gic 0 0 0 120 4>,
<0000 0 0 3 &gic 0 0 0 121 4>,
<0000 0 0 4 &gic 0 0 0 122 4>;
+ status = "disabled";
};

pcie4: pcie@3700000 {
@@ -711,6 +714,7 @@
<0000 0 0 2 &gic 0 0 0 125 4>,
<0000 0 0 3 &gic 0 0 0 126 4>,
<0000 0 0 4 &gic 0 0 0 127 4>;
+ status = "disabled";
};

sata0: sata@3200000 {
--
1.7.1



2018-10-25 11:14:40

by Bao Xiaowei

[permalink] [raw]
Subject: [PATCH 3/6] PCI: layerscape: Add the EP mode support

Add the EP mode support.

Signed-off-by: Xiaowei Bao <[email protected]>
---
.../devicetree/bindings/pci/layerscape-pci.txt | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
index 66df1e8..d3d7be1 100644
--- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
+++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
@@ -13,12 +13,15 @@ information.

Required properties:
- compatible: should contain the platform identifier such as:
+ RC mode:
"fsl,ls1021a-pcie", "snps,dw-pcie"
"fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie"
"fsl,ls2088a-pcie"
"fsl,ls1088a-pcie"
"fsl,ls1046a-pcie"
"fsl,ls1012a-pcie"
+ EP mode:
+ "fsl,ls-pcie-ep"
- reg: base addresses and lengths of the PCIe controller register blocks.
- interrupts: A list of interrupt outputs of the controller. Must contain an
entry for each entry in the interrupt-names property.
--
1.7.1


2018-10-25 11:14:43

by Bao Xiaowei

[permalink] [raw]
Subject: [PATCH 6/6] misc: pci_endpoint_test: Add the layerscape EP device support

Add the layerscape EP device support in pci_endpoint_test driver.

Signed-off-by: Xiaowei Bao <[email protected]>
---
drivers/misc/pci_endpoint_test.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 896e2df..744d10c 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -788,6 +788,8 @@ static void pci_endpoint_test_remove(struct pci_dev *pdev)
static const struct pci_device_id pci_endpoint_test_tbl[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_DRA74x) },
{ PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_DRA72x) },
+ /* 0x81c0: The device id of ls1046a in NXP. */
+ { PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, 0x81c0) },
{ PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, 0xedda) },
{ }
};
--
1.7.1


2018-10-25 11:14:48

by Bao Xiaowei

[permalink] [raw]
Subject: [PATCH 4/6] arm64: dts: Add the PCIE EP node in dts

Add the PCIE EP node in dts for ls1046a.

Signed-off-by: Xiaowei Bao <[email protected]>
---
arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 32 ++++++++++++++++++++++++
1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 64d334c..08b4f08 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -655,6 +655,17 @@
status = "disabled";
};

+ pcie_ep@3400000 {
+ compatible = "fsl,ls-pcie-ep";
+ reg = <0x00 0x03400000 0x0 0x00100000
+ 0x40 0x00000000 0x8 0x00000000>;
+ reg-names = "regs", "addr_space";
+ num-ib-windows = <6>;
+ num-ob-windows = <6>;
+ num-lanes = <2>;
+ status = "disabled";
+ };
+
pcie@3500000 {
compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
reg = <0x00 0x03500000 0x0 0x00100000 /* controller registers */
@@ -681,6 +692,17 @@
status = "disabled";
};

+ pcie_ep@3500000 {
+ compatible = "fsl,ls-pcie-ep";
+ reg = <0x00 0x03500000 0x0 0x00100000
+ 0x48 0x00000000 0x8 0x00000000>;
+ reg-names = "regs", "addr_space";
+ num-ib-windows = <6>;
+ num-ob-windows = <6>;
+ num-lanes = <2>;
+ status = "disabled";
+ };
+
pcie@3600000 {
compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
reg = <0x00 0x03600000 0x0 0x00100000 /* controller registers */
@@ -707,6 +729,16 @@
status = "disabled";
};

+ pcie_ep@3600000 {
+ compatible = "fsl,ls-pcie-ep";
+ reg = <0x00 0x03600000 0x0 0x00100000
+ 0x50 0x00000000 0x8 0x00000000>;
+ reg-names = "regs", "addr_space";
+ num-ib-windows = <6>;
+ num-ob-windows = <6>;
+ num-lanes = <2>;
+ status = "disabled";
+ };
};

reserved-memory {
--
1.7.1


2018-10-25 11:15:05

by Bao Xiaowei

[permalink] [raw]
Subject: [PATCH 2/6] ARM: dts: ls1021a: Add the status property disable PCIe

Add the status property disable the PCIe, the property will be enable
by bootloader.

Signed-off-by: Xiaowei Bao <[email protected]>
---
arch/arm/boot/dts/ls1021a.dtsi | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index bdd6e66..b769e0e 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -736,6 +736,7 @@
<0000 0 0 2 &gic GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic GIC_SPI 190 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
};

pcie@3500000 {
@@ -759,6 +760,7 @@
<0000 0 0 2 &gic GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 3 &gic GIC_SPI 191 IRQ_TYPE_LEVEL_HIGH>,
<0000 0 0 4 &gic GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
};

can0: can@2a70000 {
--
1.7.1


2018-10-25 11:15:57

by Bao Xiaowei

[permalink] [raw]
Subject: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Add the PCIe EP mode support for layerscape platform.

Signed-off-by: Xiaowei Bao <[email protected]>
---
drivers/pci/controller/dwc/Makefile | 2 +-
drivers/pci/controller/dwc/pci-layerscape-ep.c | 161 ++++++++++++++++++++++++
2 files changed, 162 insertions(+), 1 deletions(-)
create mode 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c

diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 5d2ce72..b26d617 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
-obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
+obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
new file mode 100644
index 0000000..3b33bbc
--- /dev/null
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe controller EP driver for Freescale Layerscape SoCs
+ *
+ * Copyright (C) 2018 NXP Semiconductor.
+ *
+ * Author: Xiaowei Bao <[email protected]>
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+
+#include "pcie-designware.h"
+
+#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
+
+struct ls_pcie_ep {
+ struct dw_pcie *pci;
+};
+
+#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
+
+static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie)
+{
+ struct dw_pcie *pci = pcie->pci;
+ u32 header_type;
+
+ header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
+ header_type &= 0x7f;
+
+ return header_type == PCI_HEADER_TYPE_BRIDGE;
+}
+
+static int ls_pcie_establish_link(struct dw_pcie *pci)
+{
+ return 0;
+}
+
+static const struct dw_pcie_ops ls_pcie_ep_ops = {
+ .start_link = ls_pcie_establish_link,
+};
+
+static const struct of_device_id ls_pcie_ep_of_match[] = {
+ { .compatible = "fsl,ls-pcie-ep",},
+ { },
+};
+
+static void ls_pcie_ep_init(struct dw_pcie_ep *ep)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ struct pci_epc *epc = ep->epc;
+ enum pci_barno bar;
+
+ for (bar = BAR_0; bar <= BAR_5; bar++)
+ dw_pcie_ep_reset_bar(pci, bar);
+
+ epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER;
+}
+
+static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
+ enum pci_epc_irq_type type, u16 interrupt_num)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+
+ switch (type) {
+ case PCI_EPC_IRQ_LEGACY:
+ return dw_pcie_ep_raise_legacy_irq(ep, func_no);
+ case PCI_EPC_IRQ_MSI:
+ return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
+ case PCI_EPC_IRQ_MSIX:
+ return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
+ default:
+ dev_err(pci->dev, "UNKNOWN IRQ type\n");
+ }
+
+ return 0;
+}
+
+static struct dw_pcie_ep_ops pcie_ep_ops = {
+ .ep_init = ls_pcie_ep_init,
+ .raise_irq = ls_pcie_ep_raise_irq,
+};
+
+static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
+ struct platform_device *pdev)
+{
+ struct dw_pcie *pci = pcie->pci;
+ struct device *dev = pci->dev;
+ struct dw_pcie_ep *ep;
+ struct resource *res;
+ int ret;
+
+ ep = &pci->ep;
+ ep->ops = &pcie_ep_ops;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
+ if (!res)
+ return -EINVAL;
+
+ ep->phys_base = res->start;
+ ep->addr_size = resource_size(res);
+
+ ret = dw_pcie_ep_init(ep);
+ if (ret) {
+ dev_err(dev, "failed to initialize endpoint\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int __init ls_pcie_ep_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct dw_pcie *pci;
+ struct ls_pcie_ep *pcie;
+ struct resource *dbi_base;
+ int ret;
+
+ pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+ if (!pcie)
+ return -ENOMEM;
+
+ pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
+ if (!pci)
+ return -ENOMEM;
+
+ dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
+ pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
+ if (IS_ERR(pci->dbi_base))
+ return PTR_ERR(pci->dbi_base);
+
+ pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
+ pci->dev = dev;
+ pci->ops = &ls_pcie_ep_ops;
+ pcie->pci = pci;
+
+ if (ls_pcie_is_bridge(pcie))
+ return -ENODEV;
+
+ platform_set_drvdata(pdev, pcie);
+
+ ret = ls_add_pcie_ep(pcie, pdev);
+
+ return ret;
+}
+
+static struct platform_driver ls_pcie_ep_driver = {
+ .driver = {
+ .name = "layerscape-pcie-ep",
+ .of_match_table = ls_pcie_ep_of_match,
+ .suppress_bind_attrs = true,
+ },
+};
+builtin_platform_driver_probe(ls_pcie_ep_driver, ls_pcie_ep_probe);
--
1.7.1


2018-10-25 21:53:34

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 3/6] PCI: layerscape: Add the EP mode support

On Thu, Oct 25, 2018 at 07:08:58PM +0800, Xiaowei Bao wrote:
> Add the EP mode support.
>
> Signed-off-by: Xiaowei Bao <[email protected]>
> ---
> .../devicetree/bindings/pci/layerscape-pci.txt | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 66df1e8..d3d7be1 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -13,12 +13,15 @@ information.
>
> Required properties:
> - compatible: should contain the platform identifier such as:
> + RC mode:
> "fsl,ls1021a-pcie", "snps,dw-pcie"
> "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie"
> "fsl,ls2088a-pcie"
> "fsl,ls1088a-pcie"
> "fsl,ls1046a-pcie"
> "fsl,ls1012a-pcie"
> + EP mode:
> + "fsl,ls-pcie-ep"

You need SoC specific compatibles for the same reasons as the RC.

Rob

2018-10-26 03:48:29

by Bao Xiaowei

[permalink] [raw]
Subject: RE: [PATCH 3/6] PCI: layerscape: Add the EP mode support



-----Original Message-----
From: Rob Herring <[email protected]>
Sent: 2018??10??26?? 5:53
To: Xiaowei Bao <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 3/6] PCI: layerscape: Add the EP mode support

On Thu, Oct 25, 2018 at 07:08:58PM +0800, Xiaowei Bao wrote:
> Add the EP mode support.
>
> Signed-off-by: Xiaowei Bao <[email protected]>
> ---
> .../devicetree/bindings/pci/layerscape-pci.txt | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 66df1e8..d3d7be1 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -13,12 +13,15 @@ information.
>
> Required properties:
> - compatible: should contain the platform identifier such as:
> + RC mode:
> "fsl,ls1021a-pcie", "snps,dw-pcie"
> "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie"
> "fsl,ls2088a-pcie"
> "fsl,ls1088a-pcie"
> "fsl,ls1046a-pcie"
> "fsl,ls1012a-pcie"
> + EP mode:
> + "fsl,ls-pcie-ep"

You need SoC specific compatibles for the same reasons as the RC.
[Xiaowei Bao] I want to contains all layerscape platform use one compatible if the PCIe controller work in EP mode.

Rob

2018-10-26 05:31:38

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Hi,

On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
> Add the PCIe EP mode support for layerscape platform.
>
> Signed-off-by: Xiaowei Bao <[email protected]>
> ---
> drivers/pci/controller/dwc/Makefile | 2 +-
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161 ++++++++++++++++++++++++
> 2 files changed, 162 insertions(+), 1 deletions(-)
> create mode 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 5d2ce72..b26d617 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> new file mode 100644
> index 0000000..3b33bbc
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe controller EP driver for Freescale Layerscape SoCs
> + *
> + * Copyright (C) 2018 NXP Semiconductor.
> + *
> + * Author: Xiaowei Bao <[email protected]>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/

The base address should come from dt.
> +
> +struct ls_pcie_ep {
> + struct dw_pcie *pci;
> +};
> +
> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> +
> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie)
> +{
> + struct dw_pcie *pci = pcie->pci;
> + u32 header_type;
> +
> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> + header_type &= 0x7f;
> +
> + return header_type == PCI_HEADER_TYPE_BRIDGE;
> +}
> +
> +static int ls_pcie_establish_link(struct dw_pcie *pci)
> +{
> + return 0;
> +}

There should be some way by which EP should tell RC that it is not configured
yet. Are there no bits to control LTSSM state initialization or Configuration
retry status enabling?
> +
> +static const struct dw_pcie_ops ls_pcie_ep_ops = {
> + .start_link = ls_pcie_establish_link,
> +};
> +
> +static const struct of_device_id ls_pcie_ep_of_match[] = {
> + { .compatible = "fsl,ls-pcie-ep",},
> + { },
> +};
> +
> +static void ls_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct pci_epc *epc = ep->epc;
> + enum pci_barno bar;
> +
> + for (bar = BAR_0; bar <= BAR_5; bar++)
> + dw_pcie_ep_reset_bar(pci, bar);
> +
> + epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER;
> +}
> +
> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> + enum pci_epc_irq_type type, u16 interrupt_num)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + switch (type) {
> + case PCI_EPC_IRQ_LEGACY:
> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> + case PCI_EPC_IRQ_MSI:
> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> + case PCI_EPC_IRQ_MSIX:
> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> + default:
> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
> + }
> +
> + return 0;
> +}
> +
> +static struct dw_pcie_ep_ops pcie_ep_ops = {
> + .ep_init = ls_pcie_ep_init,
> + .raise_irq = ls_pcie_ep_raise_irq,
> +};
> +
> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *pci = pcie->pci;
> + struct device *dev = pci->dev;
> + struct dw_pcie_ep *ep;
> + struct resource *res;
> + int ret;
> +
> + ep = &pci->ep;
> + ep->ops = &pcie_ep_ops;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
> + if (!res)
> + return -EINVAL;
> +
> + ep->phys_base = res->start;
> + ep->addr_size = resource_size(res);
> +
> + ret = dw_pcie_ep_init(ep);
> + if (ret) {
> + dev_err(dev, "failed to initialize endpoint\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int __init ls_pcie_ep_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dw_pcie *pci;
> + struct ls_pcie_ep *pcie;
> + struct resource *dbi_base;
> + int ret;
> +
> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> + if (!pcie)
> + return -ENOMEM;
> +
> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> + if (!pci)
> + return -ENOMEM;
> +
> + dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> + pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> + if (IS_ERR(pci->dbi_base))
> + return PTR_ERR(pci->dbi_base);
> +
> + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> + pci->dev = dev;
> + pci->ops = &ls_pcie_ep_ops;
> + pcie->pci = pci;
> +
> + if (ls_pcie_is_bridge(pcie))
> + return -ENODEV;

For an endpoint this condition should never occur. This should only mean, a
wrong compatible has been used in dt.

Thanks
Kishon

2018-10-26 07:02:24

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 3/6] PCI: layerscape: Add the EP mode support

On 10/26/18, Xiaowei Bao <[email protected]> wrote:
> From: Rob Herring <[email protected]>
>> On Thu, Oct 25, 2018 at 07:08:58PM +0800, Xiaowei Bao wrote:
>>> "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie"
>>> "fsl,ls2088a-pcie"
>>> "fsl,ls1088a-pcie"
>>> "fsl,ls1046a-pcie"
>>> "fsl,ls1012a-pcie"
>>> + EP mode:
>>> + "fsl,ls-pcie-ep"
>>
> > You need SoC specific compatibles for the same reasons as the RC.
>
> [Xiaowei Bao] I want to contains all layerscape platform use one compatible
> if the PCIe controller work in EP mode.

Do you mean only one of the SoCs that support RC mode has EP mode?
I think you still need a SoC specific compatible as Rob explained, in case
there will be a second one in the future.

If you want to ensure that you don't have to update the device driver
for each new chip that comes in when the EP mode is compatible,
the way this is handled is to list multiple values in the compatible
property, listing the first SoC that introduced the specific version of
that IP block as the most generic type, e.g.

copatible = "fsl,ls2088a-pcie-ep", "fsl,ls1012a-pcie-ep", "snps,dw-pcie-ep";

For consistency, it probably is best to match each RC mode value with
the corresponding EP mode string for each device that can support both
(if there is more than one).

Arnd

2018-10-26 07:44:40

by Bao Xiaowei

[permalink] [raw]
Subject: RE: [PATCH 3/6] PCI: layerscape: Add the EP mode support



-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Arnd Bergmann
Sent: 2018年10月26日 15:01
To: Xiaowei Bao <[email protected]>
Cc: Rob Herring <[email protected]>; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 3/6] PCI: layerscape: Add the EP mode support

On 10/26/18, Xiaowei Bao <[email protected]> wrote:
> From: Rob Herring <[email protected]>
>> On Thu, Oct 25, 2018 at 07:08:58PM +0800, Xiaowei Bao wrote:
>>> "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie"
>>> "fsl,ls2088a-pcie"
>>> "fsl,ls1088a-pcie"
>>> "fsl,ls1046a-pcie"
>>> "fsl,ls1012a-pcie
>>> + EP mode:
>>> + "fsl,ls-pcie-ep"
>>
> > You need SoC specific compatibles for the same reasons as the RC.
>
> [Xiaowei Bao] I want to contains all layerscape platform use one
> compatible if the PCIe controller work in EP mode.

Do you mean only one of the SoCs that support RC mode has EP mode?
I think you still need a SoC specific compatible as Rob explained, in case there will be a second one in the future.

If you want to ensure that you don't have to update the device driver for each new chip that comes in when the EP mode is compatible, the way this is handled is to list multiple values in the compatible property, listing the first SoC that introduced the specific version of that IP block as the most generic type, e.g.

copatible = "fsl,ls2088a-pcie-ep", "fsl,ls1012a-pcie-ep", "snps,dw-pcie-ep";

For consistency, it probably is best to match each RC mode value with the corresponding EP mode string for each device that can support both (if there is more than one).

Arnd
[Xiaowei Bao] My mean is that the ls-pcie-ep compatibles will contain all layerscape SOCs of NXP, e.g: ls1046a-pcie-ep, fsl,ls2088a-pcie-ep, ls2088a-pcie-ep and so on, other layerscape SOCs have not test except the ls1046a, I think it is compatible if the new chip or other SOCs use the DW core, OK, I will discuss this issue internally, and reply to you later.

2018-10-26 09:19:37

by Bao Xiaowei

[permalink] [raw]
Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.



-----Original Message-----
From: Kishon Vijay Abraham I <[email protected]>
Sent: 2018年10月26日 13:29
To: Xiaowei Bao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Hi,

On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
> Add the PCIe EP mode support for layerscape platform.
>
> Signed-off-by: Xiaowei Bao <[email protected]>
> ---
> drivers/pci/controller/dwc/Makefile | 2 +-
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161
> ++++++++++++++++++++++++
> 2 files changed, 162 insertions(+), 1 deletions(-) create mode
> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>
> diff --git a/drivers/pci/controller/dwc/Makefile
> b/drivers/pci/controller/dwc/Makefile
> index 5d2ce72..b26d617 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> new file mode 100644
> index 0000000..3b33bbc
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe controller EP driver for Freescale Layerscape SoCs
> + *
> + * Copyright (C) 2018 NXP Semiconductor.
> + *
> + * Author: Xiaowei Bao <[email protected]> */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/

The base address should come from dt.
> +
> +struct ls_pcie_ep {
> + struct dw_pcie *pci;
> +};
> +
> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> +
> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie) {
> + struct dw_pcie *pci = pcie->pci;
> + u32 header_type;
> +
> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> + header_type &= 0x7f;
> +
> + return header_type == PCI_HEADER_TYPE_BRIDGE; }
> +
> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
> + return 0;
> +}

There should be some way by which EP should tell RC that it is not configured yet. Are there no bits to control LTSSM state initialization or Configuration retry status enabling?
[Xiaowei Bao] There have not bits to control LTSSM state to tell the RC it is configured. The start link is auto completed.
> +
> +static const struct dw_pcie_ops ls_pcie_ep_ops = {
> + .start_link = ls_pcie_establish_link, };
> +
> +static const struct of_device_id ls_pcie_ep_of_match[] = {
> + { .compatible = "fsl,ls-pcie-ep",},
> + { },
> +};
> +
> +static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct pci_epc *epc = ep->epc;
> + enum pci_barno bar;
> +
> + for (bar = BAR_0; bar <= BAR_5; bar++)
> + dw_pcie_ep_reset_bar(pci, bar);
> +
> + epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER; }
> +
> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> + enum pci_epc_irq_type type, u16 interrupt_num) {
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + switch (type) {
> + case PCI_EPC_IRQ_LEGACY:
> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> + case PCI_EPC_IRQ_MSI:
> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> + case PCI_EPC_IRQ_MSIX:
> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> + default:
> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
> + }
> +
> + return 0;
> +}
> +
> +static struct dw_pcie_ep_ops pcie_ep_ops = {
> + .ep_init = ls_pcie_ep_init,
> + .raise_irq = ls_pcie_ep_raise_irq,
> +};
> +
> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *pci = pcie->pci;
> + struct device *dev = pci->dev;
> + struct dw_pcie_ep *ep;
> + struct resource *res;
> + int ret;
> +
> + ep = &pci->ep;
> + ep->ops = &pcie_ep_ops;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
> + if (!res)
> + return -EINVAL;
> +
> + ep->phys_base = res->start;
> + ep->addr_size = resource_size(res);
> +
> + ret = dw_pcie_ep_init(ep);
> + if (ret) {
> + dev_err(dev, "failed to initialize endpoint\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int __init ls_pcie_ep_probe(struct platform_device *pdev) {
> + struct device *dev = &pdev->dev;
> + struct dw_pcie *pci;
> + struct ls_pcie_ep *pcie;
> + struct resource *dbi_base;
> + int ret;
> +
> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> + if (!pcie)
> + return -ENOMEM;
> +
> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> + if (!pci)
> + return -ENOMEM;
> +
> + dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> + pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> + if (IS_ERR(pci->dbi_base))
> + return PTR_ERR(pci->dbi_base);
> +
> + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> + pci->dev = dev;
> + pci->ops = &ls_pcie_ep_ops;
> + pcie->pci = pci;
> +
> + if (ls_pcie_is_bridge(pcie))
> + return -ENODEV;

For an endpoint this condition should never occur. This should only mean, a wrong compatible has been used in dt.
[Xiaowei Bao] This function is a way that can check the PCI controller whether work in EP mode, I think it is more safer. Of course, it can be removed.
Thanks
Kishon

2018-10-26 20:29:25

by Leo Li

[permalink] [raw]
Subject: Re: [PATCH 3/6] PCI: layerscape: Add the EP mode support

On Fri, Oct 26, 2018 at 2:43 AM Xiaowei Bao <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: [email protected] <[email protected]> On Behalf Of Arnd Bergmann
> Sent: 2018年10月26日 15:01
> To: Xiaowei Bao <[email protected]>
> Cc: Rob Herring <[email protected]>; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 3/6] PCI: layerscape: Add the EP mode support
>
> On 10/26/18, Xiaowei Bao <[email protected]> wrote:
> > From: Rob Herring <[email protected]>
> >> On Thu, Oct 25, 2018 at 07:08:58PM +0800, Xiaowei Bao wrote:
> >>> "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie"
> >>> "fsl,ls2088a-pcie"
> >>> "fsl,ls1088a-pcie"
> >>> "fsl,ls1046a-pcie"
> >>> "fsl,ls1012a-pcie
> >>> + EP mode:
> >>> + "fsl,ls-pcie-ep"
> >>
> > > You need SoC specific compatibles for the same reasons as the RC.
> >
> > [Xiaowei Bao] I want to contains all layerscape platform use one
> > compatible if the PCIe controller work in EP mode.
>
> Do you mean only one of the SoCs that support RC mode has EP mode?
> I think you still need a SoC specific compatible as Rob explained, in case there will be a second one in the future.
>
> If you want to ensure that you don't have to update the device driver for each new chip that comes in when the EP mode is compatible, the way this is handled is to list multiple values in the compatible property, listing the first SoC that introduced the specific version of that IP block as the most generic type, e.g.
>
> copatible = "fsl,ls2088a-pcie-ep", "fsl,ls1012a-pcie-ep", "snps,dw-pcie-ep";
>
> For consistency, it probably is best to match each RC mode value with the corresponding EP mode string for each device that can support both (if there is more than one).
>
> Arnd
> [Xiaowei Bao] My mean is that the ls-pcie-ep compatibles will contain all layerscape SOCs of NXP, e.g: ls1046a-pcie-ep, fsl,ls2088a-pcie-ep, ls2088a-pcie-ep and so on, other layerscape SOCs have not test except the ls1046a, I think it is compatible if the new chip or other SOCs use the DW core, OK, I will discuss this issue internally, and reply to you later.

You can define a generic compatible string for the EP mode of all
these platforms. But like Rob and Arnd mentioned, it is good to also
define the SoC specific compatible strings just in case that we need
special treatment for certain SoCs in the future.

Regards,
Leo

2018-10-29 02:41:40

by Bao Xiaowei

[permalink] [raw]
Subject: RE: [PATCH 3/6] PCI: layerscape: Add the EP mode support



-----Original Message-----
From: Li Yang <[email protected]>
Sent: 2018年10月27日 4:29
To: Xiaowei Bao <[email protected]>
Cc: Arnd Bergmann <[email protected]>; Rob Herring <[email protected]>; Bjorn Helgaas <[email protected]>; Mark Rutland <[email protected]>; Shawn Guo <[email protected]>; [email protected]; [email protected]; Greg Kroah-Hartman <[email protected]>; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; Kate Stewart <[email protected]>; [email protected]; Philippe Ombredanne <[email protected]>; [email protected]; [email protected]; [email protected]; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <[email protected]>; lkml <[email protected]>; moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE <[email protected]>; linuxppc-dev <[email protected]>
Subject: Re: [PATCH 3/6] PCI: layerscape: Add the EP mode support

On Fri, Oct 26, 2018 at 2:43 AM Xiaowei Bao <[email protected]> wrote:
>
>
>
> -----Original Message-----
> From: [email protected] <[email protected]> On Behalf Of
> Arnd Bergmann
> Sent: 2018年10月26日 15:01
> To: Xiaowei Bao <[email protected]>
> Cc: Rob Herring <[email protected]>; [email protected];
> [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; [email protected];
> [email protected]; M.h. Lian <[email protected]>; Mingkai
> Hu <[email protected]>; Roy Zang <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 3/6] PCI: layerscape: Add the EP mode support
>
> On 10/26/18, Xiaowei Bao <[email protected]> wrote:
> > From: Rob Herring <[email protected]>
> >> On Thu, Oct 25, 2018 at 07:08:58PM +0800, Xiaowei Bao wrote:
> >>> "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie"
> >>> "fsl,ls2088a-pcie"
> >>> "fsl,ls1088a-pcie"
> >>> "fsl,ls1046a-pcie"
> >>> "fsl,ls1012a-pcie
> >>> + EP mode:
> >>> + "fsl,ls-pcie-ep"
> >>
> > > You need SoC specific compatibles for the same reasons as the RC.
> >
> > [Xiaowei Bao] I want to contains all layerscape platform use one
> > compatible if the PCIe controller work in EP mode.
>
> Do you mean only one of the SoCs that support RC mode has EP mode?
> I think you still need a SoC specific compatible as Rob explained, in case there will be a second one in the future.
>
> If you want to ensure that you don't have to update the device driver for each new chip that comes in when the EP mode is compatible, the way this is handled is to list multiple values in the compatible property, listing the first SoC that introduced the specific version of that IP block as the most generic type, e.g.
>
> copatible = "fsl,ls2088a-pcie-ep", "fsl,ls1012a-pcie-ep",
> "snps,dw-pcie-ep";
>
> For consistency, it probably is best to match each RC mode value with the corresponding EP mode string for each device that can support both (if there is more than one).
>
> Arnd
> [Xiaowei Bao] My mean is that the ls-pcie-ep compatibles will contain all layerscape SOCs of NXP, e.g: ls1046a-pcie-ep, fsl,ls2088a-pcie-ep, ls2088a-pcie-ep and so on, other layerscape SOCs have not test except the ls1046a, I think it is compatible if the new chip or other SOCs use the DW core, OK, I will discuss this issue internally, and reply to you later.

You can define a generic compatible string for the EP mode of all these platforms. But like Rob and Arnd mentioned, it is good to also define the SoC specific compatible strings just in case that we need special treatment for certain SoCs in the future.

Regards,
Leo

[Xiaowei Bao] Hi Leo, OK, I will add the SoC specific compatible strings in patch-v2, thanks a lot.

2018-10-31 02:34:32

by Bao Xiaowei

[permalink] [raw]
Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.



-----Original Message-----
From: Xiaowei Bao
Sent: 2018年10月26日 17:19
To: 'Kishon Vijay Abraham I' <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Jiafei Pan <[email protected]>
Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.



-----Original Message-----
From: Kishon Vijay Abraham I <[email protected]>
Sent: 2018年10月26日 13:29
To: Xiaowei Bao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Hi,

On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
> Add the PCIe EP mode support for layerscape platform.
>
> Signed-off-by: Xiaowei Bao <[email protected]>
> ---
> drivers/pci/controller/dwc/Makefile | 2 +-
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161
> ++++++++++++++++++++++++
> 2 files changed, 162 insertions(+), 1 deletions(-) create mode
> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>
> diff --git a/drivers/pci/controller/dwc/Makefile
> b/drivers/pci/controller/dwc/Makefile
> index 5d2ce72..b26d617 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> new file mode 100644
> index 0000000..3b33bbc
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe controller EP driver for Freescale Layerscape SoCs
> + *
> + * Copyright (C) 2018 NXP Semiconductor.
> + *
> + * Author: Xiaowei Bao <[email protected]> */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/

The base address should come from dt.
> +
> +struct ls_pcie_ep {
> + struct dw_pcie *pci;
> +};
> +
> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> +
> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie) {
> + struct dw_pcie *pci = pcie->pci;
> + u32 header_type;
> +
> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> + header_type &= 0x7f;
> +
> + return header_type == PCI_HEADER_TYPE_BRIDGE; }
> +
> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
> + return 0;
> +}

There should be some way by which EP should tell RC that it is not configured yet. Are there no bits to control LTSSM state initialization or Configuration retry status enabling?
[Xiaowei Bao] There have not bits to control LTSSM state to tell the RC it is configured. The start link is auto completed.
[Xiaowei Bao] Hi Kishon, is there any advice?
> +
> +static const struct dw_pcie_ops ls_pcie_ep_ops = {
> + .start_link = ls_pcie_establish_link, };
> +
> +static const struct of_device_id ls_pcie_ep_of_match[] = {
> + { .compatible = "fsl,ls-pcie-ep",},
> + { },
> +};
> +
> +static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct pci_epc *epc = ep->epc;
> + enum pci_barno bar;
> +
> + for (bar = BAR_0; bar <= BAR_5; bar++)
> + dw_pcie_ep_reset_bar(pci, bar);
> +
> + epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER; }
> +
> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> + enum pci_epc_irq_type type, u16 interrupt_num) {
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + switch (type) {
> + case PCI_EPC_IRQ_LEGACY:
> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> + case PCI_EPC_IRQ_MSI:
> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> + case PCI_EPC_IRQ_MSIX:
> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> + default:
> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
> + }
> +
> + return 0;
> +}
> +
> +static struct dw_pcie_ep_ops pcie_ep_ops = {
> + .ep_init = ls_pcie_ep_init,
> + .raise_irq = ls_pcie_ep_raise_irq,
> +};
> +
> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *pci = pcie->pci;
> + struct device *dev = pci->dev;
> + struct dw_pcie_ep *ep;
> + struct resource *res;
> + int ret;
> +
> + ep = &pci->ep;
> + ep->ops = &pcie_ep_ops;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
> + if (!res)
> + return -EINVAL;
> +
> + ep->phys_base = res->start;
> + ep->addr_size = resource_size(res);
> +
> + ret = dw_pcie_ep_init(ep);
> + if (ret) {
> + dev_err(dev, "failed to initialize endpoint\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int __init ls_pcie_ep_probe(struct platform_device *pdev) {
> + struct device *dev = &pdev->dev;
> + struct dw_pcie *pci;
> + struct ls_pcie_ep *pcie;
> + struct resource *dbi_base;
> + int ret;
> +
> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> + if (!pcie)
> + return -ENOMEM;
> +
> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> + if (!pci)
> + return -ENOMEM;
> +
> + dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> + pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> + if (IS_ERR(pci->dbi_base))
> + return PTR_ERR(pci->dbi_base);
> +
> + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> + pci->dev = dev;
> + pci->ops = &ls_pcie_ep_ops;
> + pcie->pci = pci;
> +
> + if (ls_pcie_is_bridge(pcie))
> + return -ENODEV;

For an endpoint this condition should never occur. This should only mean, a wrong compatible has been used in dt.
[Xiaowei Bao] This function is a way that can check the PCI controller whether work in EP mode, I think it is more safer. Of course, it can be removed.
[Xiaowei Bao] Hi Kishon, is there any advice?
Thanks
Kishon

2018-10-31 04:22:27

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Hi,

On 31/10/18 8:03 AM, Xiaowei Bao wrote:
>
>
> -----Original Message-----
> From: Xiaowei Bao
> Sent: 2018年10月26日 17:19
> To: 'Kishon Vijay Abraham I' <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Cc: Jiafei Pan <[email protected]>
> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>
>
>
> -----Original Message-----
> From: Kishon Vijay Abraham I <[email protected]>
> Sent: 2018年10月26日 13:29
> To: Xiaowei Bao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>
> Hi,
>
> On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
>> Add the PCIe EP mode support for layerscape platform.
>>
>> Signed-off-by: Xiaowei Bao <[email protected]>
>> ---
>> drivers/pci/controller/dwc/Makefile | 2 +-
>> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161
>> ++++++++++++++++++++++++
>> 2 files changed, 162 insertions(+), 1 deletions(-) create mode
>> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>>
>> diff --git a/drivers/pci/controller/dwc/Makefile
>> b/drivers/pci/controller/dwc/Makefile
>> index 5d2ce72..b26d617 100644
>> --- a/drivers/pci/controller/dwc/Makefile
>> +++ b/drivers/pci/controller/dwc/Makefile
>> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
>> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
>> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>> new file mode 100644
>> index 0000000..3b33bbc
>> --- /dev/null
>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>> @@ -0,0 +1,161 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PCIe controller EP driver for Freescale Layerscape SoCs
>> + *
>> + * Copyright (C) 2018 NXP Semiconductor.
>> + *
>> + * Author: Xiaowei Bao <[email protected]> */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_address.h>
>> +#include <linux/pci.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/resource.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
>
> The base address should come from dt.
>> +
>> +struct ls_pcie_ep {
>> + struct dw_pcie *pci;
>> +};
>> +
>> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
>> +
>> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie) {
>> + struct dw_pcie *pci = pcie->pci;
>> + u32 header_type;
>> +
>> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
>> + header_type &= 0x7f;
>> +
>> + return header_type == PCI_HEADER_TYPE_BRIDGE; }
>> +
>> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
>> + return 0;
>> +}
>
> There should be some way by which EP should tell RC that it is not configured yet. Are there no bits to control LTSSM state initialization or Configuration retry status enabling?
> [Xiaowei Bao] There have not bits to control LTSSM state to tell the RC it is configured. The start link is auto completed.
> [Xiaowei Bao] Hi Kishon, is there any advice?

If there is no HW support, I don't think anything could be done here. This
could result in RC reading configuration space even before EP is fully initialized.
>> +
>> +static const struct dw_pcie_ops ls_pcie_ep_ops = {
>> + .start_link = ls_pcie_establish_link, };
>> +
>> +static const struct of_device_id ls_pcie_ep_of_match[] = {
>> + { .compatible = "fsl,ls-pcie-ep",},
>> + { },
>> +};
>> +
>> +static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> + struct pci_epc *epc = ep->epc;
>> + enum pci_barno bar;
>> +
>> + for (bar = BAR_0; bar <= BAR_5; bar++)
>> + dw_pcie_ep_reset_bar(pci, bar);
>> +
>> + epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER; }
>> +
>> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>> + enum pci_epc_irq_type type, u16 interrupt_num) {
>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +
>> + switch (type) {
>> + case PCI_EPC_IRQ_LEGACY:
>> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
>> + case PCI_EPC_IRQ_MSI:
>> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
>> + case PCI_EPC_IRQ_MSIX:
>> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
>> + default:
>> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static struct dw_pcie_ep_ops pcie_ep_ops = {
>> + .ep_init = ls_pcie_ep_init,
>> + .raise_irq = ls_pcie_ep_raise_irq,
>> +};
>> +
>> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
>> + struct platform_device *pdev)
>> +{
>> + struct dw_pcie *pci = pcie->pci;
>> + struct device *dev = pci->dev;
>> + struct dw_pcie_ep *ep;
>> + struct resource *res;
>> + int ret;
>> +
>> + ep = &pci->ep;
>> + ep->ops = &pcie_ep_ops;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
>> + if (!res)
>> + return -EINVAL;
>> +
>> + ep->phys_base = res->start;
>> + ep->addr_size = resource_size(res);
>> +
>> + ret = dw_pcie_ep_init(ep);
>> + if (ret) {
>> + dev_err(dev, "failed to initialize endpoint\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __init ls_pcie_ep_probe(struct platform_device *pdev) {
>> + struct device *dev = &pdev->dev;
>> + struct dw_pcie *pci;
>> + struct ls_pcie_ep *pcie;
>> + struct resource *dbi_base;
>> + int ret;
>> +
>> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>> + if (!pcie)
>> + return -ENOMEM;
>> +
>> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>> + if (!pci)
>> + return -ENOMEM;
>> +
>> + dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
>> + pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
>> + if (IS_ERR(pci->dbi_base))
>> + return PTR_ERR(pci->dbi_base);
>> +
>> + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
>> + pci->dev = dev;
>> + pci->ops = &ls_pcie_ep_ops;
>> + pcie->pci = pci;
>> +
>> + if (ls_pcie_is_bridge(pcie))
>> + return -ENODEV;
>
> For an endpoint this condition should never occur. This should only mean, a wrong compatible has been used in dt.
> [Xiaowei Bao] This function is a way that can check the PCI controller whether work in EP mode, I think it is more safer. Of course, it can be removed.
> [Xiaowei Bao] Hi Kishon, is there any advice?

IMHO this check is not required.

Thanks
Kishon

2018-10-31 10:39:33

by Bao Xiaowei

[permalink] [raw]
Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.



-----Original Message-----
From: Kishon Vijay Abraham I <[email protected]>
Sent: 2018年10月31日 12:15
To: Xiaowei Bao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Jiafei Pan <[email protected]>
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Hi,

On 31/10/18 8:03 AM, Xiaowei Bao wrote:
>
>
> -----Original Message-----
> From: Xiaowei Bao
> Sent: 2018年10月26日 17:19
> To: 'Kishon Vijay Abraham I' <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; [email protected];
> [email protected]; M.h. Lian <[email protected]>; Mingkai
> Hu <[email protected]>; Roy Zang <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Jiafei Pan <[email protected]>
> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>
>
>
> -----Original Message-----
> From: Kishon Vijay Abraham I <[email protected]>
> Sent: 2018年10月26日 13:29
> To: Xiaowei Bao <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; [email protected];
> [email protected]; M.h. Lian <[email protected]>; Mingkai
> Hu <[email protected]>; Roy Zang <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>
> Hi,
>
> On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
>> Add the PCIe EP mode support for layerscape platform.
>>
>> Signed-off-by: Xiaowei Bao <[email protected]>
>> ---
>> drivers/pci/controller/dwc/Makefile | 2 +-
>> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161
>> ++++++++++++++++++++++++
>> 2 files changed, 162 insertions(+), 1 deletions(-) create mode
>> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>>
>> diff --git a/drivers/pci/controller/dwc/Makefile
>> b/drivers/pci/controller/dwc/Makefile
>> index 5d2ce72..b26d617 100644
>> --- a/drivers/pci/controller/dwc/Makefile
>> +++ b/drivers/pci/controller/dwc/Makefile
>> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
>> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
>> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>> new file mode 100644
>> index 0000000..3b33bbc
>> --- /dev/null
>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>> @@ -0,0 +1,161 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PCIe controller EP driver for Freescale Layerscape SoCs
>> + *
>> + * Copyright (C) 2018 NXP Semiconductor.
>> + *
>> + * Author: Xiaowei Bao <[email protected]> */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_address.h>
>> +#include <linux/pci.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/resource.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
>
> The base address should come from dt.
>> +
>> +struct ls_pcie_ep {
>> + struct dw_pcie *pci;
>> +};
>> +
>> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
>> +
>> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie) {
>> + struct dw_pcie *pci = pcie->pci;
>> + u32 header_type;
>> +
>> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
>> + header_type &= 0x7f;
>> +
>> + return header_type == PCI_HEADER_TYPE_BRIDGE; }
>> +
>> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
>> + return 0;
>> +}
>
> There should be some way by which EP should tell RC that it is not configured yet. Are there no bits to control LTSSM state initialization or Configuration retry status enabling?
> [Xiaowei Bao] There have not bits to control LTSSM state to tell the RC it is configured. The start link is auto completed.
> [Xiaowei Bao] Hi Kishon, is there any advice?

If there is no HW support, I don't think anything could be done here. This could result in RC reading configuration space even before EP is fully initialized.
[Xiaowei Bao] The bootloader have initialized the EP device and set the config ready bit, and the kernel don't need to do anything.
>> +
>> +static const struct dw_pcie_ops ls_pcie_ep_ops = {
>> + .start_link = ls_pcie_establish_link, };
>> +
>> +static const struct of_device_id ls_pcie_ep_of_match[] = {
>> + { .compatible = "fsl,ls-pcie-ep",},
>> + { },
>> +};
>> +
>> +static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> + struct pci_epc *epc = ep->epc;
>> + enum pci_barno bar;
>> +
>> + for (bar = BAR_0; bar <= BAR_5; bar++)
>> + dw_pcie_ep_reset_bar(pci, bar);
>> +
>> + epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER; }
>> +
>> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>> + enum pci_epc_irq_type type, u16 interrupt_num) {
>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +
>> + switch (type) {
>> + case PCI_EPC_IRQ_LEGACY:
>> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
>> + case PCI_EPC_IRQ_MSI:
>> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
>> + case PCI_EPC_IRQ_MSIX:
>> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
>> + default:
>> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static struct dw_pcie_ep_ops pcie_ep_ops = {
>> + .ep_init = ls_pcie_ep_init,
>> + .raise_irq = ls_pcie_ep_raise_irq,
>> +};
>> +
>> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
>> + struct platform_device *pdev)
>> +{
>> + struct dw_pcie *pci = pcie->pci;
>> + struct device *dev = pci->dev;
>> + struct dw_pcie_ep *ep;
>> + struct resource *res;
>> + int ret;
>> +
>> + ep = &pci->ep;
>> + ep->ops = &pcie_ep_ops;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
>> + if (!res)
>> + return -EINVAL;
>> +
>> + ep->phys_base = res->start;
>> + ep->addr_size = resource_size(res);
>> +
>> + ret = dw_pcie_ep_init(ep);
>> + if (ret) {
>> + dev_err(dev, "failed to initialize endpoint\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __init ls_pcie_ep_probe(struct platform_device *pdev) {
>> + struct device *dev = &pdev->dev;
>> + struct dw_pcie *pci;
>> + struct ls_pcie_ep *pcie;
>> + struct resource *dbi_base;
>> + int ret;
>> +
>> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>> + if (!pcie)
>> + return -ENOMEM;
>> +
>> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>> + if (!pci)
>> + return -ENOMEM;
>> +
>> + dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
>> + pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
>> + if (IS_ERR(pci->dbi_base))
>> + return PTR_ERR(pci->dbi_base);
>> +
>> + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
>> + pci->dev = dev;
>> + pci->ops = &ls_pcie_ep_ops;
>> + pcie->pci = pci;
>> +
>> + if (ls_pcie_is_bridge(pcie))
>> + return -ENODEV;
>
> For an endpoint this condition should never occur. This should only mean, a wrong compatible has been used in dt.
> [Xiaowei Bao] This function is a way that can check the PCI controller whether work in EP mode, I think it is more safer. Of course, it can be removed.
> [Xiaowei Bao] Hi Kishon, is there any advice?

IMHO this check is not required.
[Xiaowei Bao] OK, I will remove this function in patch-v2.

Thanks
Kishon

2018-11-05 08:58:56

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Hi,

On 31/10/18 4:08 PM, Xiaowei Bao wrote:
>
>
> -----Original Message-----
> From: Kishon Vijay Abraham I <[email protected]>
> Sent: 2018年10月31日 12:15
> To: Xiaowei Bao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Cc: Jiafei Pan <[email protected]>
> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>
> Hi,
>
> On 31/10/18 8:03 AM, Xiaowei Bao wrote:
>>
>>
>> -----Original Message-----
>> From: Xiaowei Bao
>> Sent: 2018年10月26日 17:19
>> To: 'Kishon Vijay Abraham I' <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected]; Leo Li
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; M.h. Lian <[email protected]>; Mingkai
>> Hu <[email protected]>; Roy Zang <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Cc: Jiafei Pan <[email protected]>
>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>
>>
>>
>> -----Original Message-----
>> From: Kishon Vijay Abraham I <[email protected]>
>> Sent: 2018年10月26日 13:29
>> To: Xiaowei Bao <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected]; Leo Li
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; M.h. Lian <[email protected]>; Mingkai
>> Hu <[email protected]>; Roy Zang <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>
>> Hi,
>>
>> On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
>>> Add the PCIe EP mode support for layerscape platform.
>>>
>>> Signed-off-by: Xiaowei Bao <[email protected]>
>>> ---
>>> drivers/pci/controller/dwc/Makefile | 2 +-
>>> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161
>>> ++++++++++++++++++++++++
>>> 2 files changed, 162 insertions(+), 1 deletions(-) create mode
>>> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>
>>> diff --git a/drivers/pci/controller/dwc/Makefile
>>> b/drivers/pci/controller/dwc/Makefile
>>> index 5d2ce72..b26d617 100644
>>> --- a/drivers/pci/controller/dwc/Makefile
>>> +++ b/drivers/pci/controller/dwc/Makefile
>>> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>>> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>>> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>>> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
>>> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>>> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
>>> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>> new file mode 100644
>>> index 0000000..3b33bbc
>>> --- /dev/null
>>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>> @@ -0,0 +1,161 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * PCIe controller EP driver for Freescale Layerscape SoCs
>>> + *
>>> + * Copyright (C) 2018 NXP Semiconductor.
>>> + *
>>> + * Author: Xiaowei Bao <[email protected]> */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/init.h>
>>> +#include <linux/of_pci.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/pci.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/resource.h>
>>> +
>>> +#include "pcie-designware.h"
>>> +
>>> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
>>
>> The base address should come from dt.
>>> +
>>> +struct ls_pcie_ep {
>>> + struct dw_pcie *pci;
>>> +};
>>> +
>>> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
>>> +
>>> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie) {
>>> + struct dw_pcie *pci = pcie->pci;
>>> + u32 header_type;
>>> +
>>> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
>>> + header_type &= 0x7f;
>>> +
>>> + return header_type == PCI_HEADER_TYPE_BRIDGE; }
>>> +
>>> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
>>> + return 0;
>>> +}
>>
>> There should be some way by which EP should tell RC that it is not configured yet. Are there no bits to control LTSSM state initialization or Configuration retry status enabling?
>> [Xiaowei Bao] There have not bits to control LTSSM state to tell the RC it is configured. The start link is auto completed.
>> [Xiaowei Bao] Hi Kishon, is there any advice?
>
> If there is no HW support, I don't think anything could be done here. This could result in RC reading configuration space even before EP is fully initialized.
> [Xiaowei Bao] The bootloader have initialized the EP device and set the config ready bit, and the kernel don't need to do anything.

What does bootloader initalize? The EP driver here will reinitialize everything
right?

Thanks
Kishon
>>> +
>>> +static const struct dw_pcie_ops ls_pcie_ep_ops = {
>>> + .start_link = ls_pcie_establish_link, };
>>> +
>>> +static const struct of_device_id ls_pcie_ep_of_match[] = {
>>> + { .compatible = "fsl,ls-pcie-ep",},
>>> + { },
>>> +};
>>> +
>>> +static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
>>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>> + struct pci_epc *epc = ep->epc;
>>> + enum pci_barno bar;
>>> +
>>> + for (bar = BAR_0; bar <= BAR_5; bar++)
>>> + dw_pcie_ep_reset_bar(pci, bar);
>>> +
>>> + epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER; }
>>> +
>>> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>> + enum pci_epc_irq_type type, u16 interrupt_num) {
>>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>> +
>>> + switch (type) {
>>> + case PCI_EPC_IRQ_LEGACY:
>>> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
>>> + case PCI_EPC_IRQ_MSI:
>>> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
>>> + case PCI_EPC_IRQ_MSIX:
>>> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
>>> + default:
>>> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct dw_pcie_ep_ops pcie_ep_ops = {
>>> + .ep_init = ls_pcie_ep_init,
>>> + .raise_irq = ls_pcie_ep_raise_irq,
>>> +};
>>> +
>>> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
>>> + struct platform_device *pdev)
>>> +{
>>> + struct dw_pcie *pci = pcie->pci;
>>> + struct device *dev = pci->dev;
>>> + struct dw_pcie_ep *ep;
>>> + struct resource *res;
>>> + int ret;
>>> +
>>> + ep = &pci->ep;
>>> + ep->ops = &pcie_ep_ops;
>>> +
>>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
>>> + if (!res)
>>> + return -EINVAL;
>>> +
>>> + ep->phys_base = res->start;
>>> + ep->addr_size = resource_size(res);
>>> +
>>> + ret = dw_pcie_ep_init(ep);
>>> + if (ret) {
>>> + dev_err(dev, "failed to initialize endpoint\n");
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __init ls_pcie_ep_probe(struct platform_device *pdev) {
>>> + struct device *dev = &pdev->dev;
>>> + struct dw_pcie *pci;
>>> + struct ls_pcie_ep *pcie;
>>> + struct resource *dbi_base;
>>> + int ret;
>>> +
>>> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>>> + if (!pcie)
>>> + return -ENOMEM;
>>> +
>>> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>>> + if (!pci)
>>> + return -ENOMEM;
>>> +
>>> + dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
>>> + pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
>>> + if (IS_ERR(pci->dbi_base))
>>> + return PTR_ERR(pci->dbi_base);
>>> +
>>> + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
>>> + pci->dev = dev;
>>> + pci->ops = &ls_pcie_ep_ops;
>>> + pcie->pci = pci;
>>> +
>>> + if (ls_pcie_is_bridge(pcie))
>>> + return -ENODEV;
>>
>> For an endpoint this condition should never occur. This should only mean, a wrong compatible has been used in dt.
>> [Xiaowei Bao] This function is a way that can check the PCI controller whether work in EP mode, I think it is more safer. Of course, it can be removed.
>> [Xiaowei Bao] Hi Kishon, is there any advice?
>
> IMHO this check is not required.
> [Xiaowei Bao] OK, I will remove this function in patch-v2.
>
> Thanks
> Kishon
>

2018-11-05 09:16:29

by Bao Xiaowei

[permalink] [raw]
Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.



-----Original Message-----
From: Kishon Vijay Abraham I <[email protected]>
Sent: 2018年11月5日 16:57
To: Xiaowei Bao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Jiafei Pan <[email protected]>
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Hi,

On 31/10/18 4:08 PM, Xiaowei Bao wrote:
>
>
> -----Original Message-----
> From: Kishon Vijay Abraham I <[email protected]>
> Sent: 2018年10月31日 12:15
> To: Xiaowei Bao <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; [email protected];
> [email protected]; M.h. Lian <[email protected]>; Mingkai
> Hu <[email protected]>; Roy Zang <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Jiafei Pan <[email protected]>
> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>
> Hi,
>
> On 31/10/18 8:03 AM, Xiaowei Bao wrote:
>>
>>
>> -----Original Message-----
>> From: Xiaowei Bao
>> Sent: 2018年10月26日 17:19
>> To: 'Kishon Vijay Abraham I' <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected]; Leo Li
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; M.h. Lian <[email protected]>;
>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Cc: Jiafei Pan <[email protected]>
>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>
>>
>>
>> -----Original Message-----
>> From: Kishon Vijay Abraham I <[email protected]>
>> Sent: 2018年10月26日 13:29
>> To: Xiaowei Bao <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected]; Leo Li
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; M.h. Lian <[email protected]>;
>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>
>> Hi,
>>
>> On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
>>> Add the PCIe EP mode support for layerscape platform.
>>>
>>> Signed-off-by: Xiaowei Bao <[email protected]>
>>> ---
>>> drivers/pci/controller/dwc/Makefile | 2 +-
>>> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161
>>> ++++++++++++++++++++++++
>>> 2 files changed, 162 insertions(+), 1 deletions(-) create mode
>>> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>
>>> diff --git a/drivers/pci/controller/dwc/Makefile
>>> b/drivers/pci/controller/dwc/Makefile
>>> index 5d2ce72..b26d617 100644
>>> --- a/drivers/pci/controller/dwc/Makefile
>>> +++ b/drivers/pci/controller/dwc/Makefile
>>> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>>> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>>> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>>> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>> +pci-layerscape-ep.o
>>> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>>> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
>>> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>> new file mode 100644
>>> index 0000000..3b33bbc
>>> --- /dev/null
>>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>> @@ -0,0 +1,161 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * PCIe controller EP driver for Freescale Layerscape SoCs
>>> + *
>>> + * Copyright (C) 2018 NXP Semiconductor.
>>> + *
>>> + * Author: Xiaowei Bao <[email protected]> */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/init.h>
>>> +#include <linux/of_pci.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/pci.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/resource.h>
>>> +
>>> +#include "pcie-designware.h"
>>> +
>>> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
>>
>> The base address should come from dt.
>>> +
>>> +struct ls_pcie_ep {
>>> + struct dw_pcie *pci;
>>> +};
>>> +
>>> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
>>> +
>>> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie) {
>>> + struct dw_pcie *pci = pcie->pci;
>>> + u32 header_type;
>>> +
>>> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
>>> + header_type &= 0x7f;
>>> +
>>> + return header_type == PCI_HEADER_TYPE_BRIDGE; }
>>> +
>>> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
>>> + return 0;
>>> +}
>>
>> There should be some way by which EP should tell RC that it is not configured yet. Are there no bits to control LTSSM state initialization or Configuration retry status enabling?
>> [Xiaowei Bao] There have not bits to control LTSSM state to tell the RC it is configured. The start link is auto completed.
>> [Xiaowei Bao] Hi Kishon, is there any advice?
>
> If there is no HW support, I don't think anything could be done here. This could result in RC reading configuration space even before EP is fully initialized.
> [Xiaowei Bao] The bootloader have initialized the EP device and set the config ready bit, and the kernel don't need to do anything.

What does bootloader initalize? The EP driver here will reinitialize everything right?
[Xiaowei Bao] The bootloader initialize BAR size, outbound window, inbound window and set the config ready bit.
yes, the EP framework will reinitialize, if don't set the dw_pcie_ops there will have call trace, because the DW driver will call the write_dbi or read_dbi function directly but not check the dw_pcie_ops whether is null. I refer to the pcie-designware- plat.c to implement it. I don't know what you said about this, please explain detailly, Thanks a lot.

Thanks
Kishon
>>> +
>>> +static const struct dw_pcie_ops ls_pcie_ep_ops = {
>>> + .start_link = ls_pcie_establish_link, };
>>> +
>>> +static const struct of_device_id ls_pcie_ep_of_match[] = {
>>> + { .compatible = "fsl,ls-pcie-ep",},
>>> + { },
>>> +};
>>> +
>>> +static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
>>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>> + struct pci_epc *epc = ep->epc;
>>> + enum pci_barno bar;
>>> +
>>> + for (bar = BAR_0; bar <= BAR_5; bar++)
>>> + dw_pcie_ep_reset_bar(pci, bar);
>>> +
>>> + epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER; }
>>> +
>>> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>> + enum pci_epc_irq_type type, u16 interrupt_num) {
>>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>> +
>>> + switch (type) {
>>> + case PCI_EPC_IRQ_LEGACY:
>>> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
>>> + case PCI_EPC_IRQ_MSI:
>>> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
>>> + case PCI_EPC_IRQ_MSIX:
>>> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
>>> + default:
>>> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct dw_pcie_ep_ops pcie_ep_ops = {
>>> + .ep_init = ls_pcie_ep_init,
>>> + .raise_irq = ls_pcie_ep_raise_irq, };
>>> +
>>> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
>>> + struct platform_device *pdev)
>>> +{
>>> + struct dw_pcie *pci = pcie->pci;
>>> + struct device *dev = pci->dev;
>>> + struct dw_pcie_ep *ep;
>>> + struct resource *res;
>>> + int ret;
>>> +
>>> + ep = &pci->ep;
>>> + ep->ops = &pcie_ep_ops;
>>> +
>>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
>>> + if (!res)
>>> + return -EINVAL;
>>> +
>>> + ep->phys_base = res->start;
>>> + ep->addr_size = resource_size(res);
>>> +
>>> + ret = dw_pcie_ep_init(ep);
>>> + if (ret) {
>>> + dev_err(dev, "failed to initialize endpoint\n");
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __init ls_pcie_ep_probe(struct platform_device *pdev) {
>>> + struct device *dev = &pdev->dev;
>>> + struct dw_pcie *pci;
>>> + struct ls_pcie_ep *pcie;
>>> + struct resource *dbi_base;
>>> + int ret;
>>> +
>>> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>>> + if (!pcie)
>>> + return -ENOMEM;
>>> +
>>> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>>> + if (!pci)
>>> + return -ENOMEM;
>>> +
>>> + dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
>>> + pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
>>> + if (IS_ERR(pci->dbi_base))
>>> + return PTR_ERR(pci->dbi_base);
>>> +
>>> + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
>>> + pci->dev = dev;
>>> + pci->ops = &ls_pcie_ep_ops;
>>> + pcie->pci = pci;
>>> +
>>> + if (ls_pcie_is_bridge(pcie))
>>> + return -ENODEV;
>>
>> For an endpoint this condition should never occur. This should only mean, a wrong compatible has been used in dt.
>> [Xiaowei Bao] This function is a way that can check the PCI controller whether work in EP mode, I think it is more safer. Of course, it can be removed.
>> [Xiaowei Bao] Hi Kishon, is there any advice?
>
> IMHO this check is not required.
> [Xiaowei Bao] OK, I will remove this function in patch-v2.
>
> Thanks
> Kishon
>

2018-11-06 06:09:04

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

(Removed Niklas as mails to him is bouncing)

Hi,

Please fix your email client. Refer Documentation/process/email-clients.rst

On 05/11/18 2:45 PM, Xiaowei Bao wrote:
>
>
> -----Original Message-----
> From: Kishon Vijay Abraham I <[email protected]>
> Sent: 2018年11月5日 16:57
> To: Xiaowei Bao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Cc: Jiafei Pan <[email protected]>
> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>
> Hi,
>
> On 31/10/18 4:08 PM, Xiaowei Bao wrote:
>>
>>
>> -----Original Message-----
>> From: Kishon Vijay Abraham I <[email protected]>
>> Sent: 2018年10月31日 12:15
>> To: Xiaowei Bao <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected]; Leo Li
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; M.h. Lian <[email protected]>; Mingkai
>> Hu <[email protected]>; Roy Zang <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Cc: Jiafei Pan <[email protected]>
>> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>
>> Hi,
>>
>> On 31/10/18 8:03 AM, Xiaowei Bao wrote:
>>>
>>>
>>> -----Original Message-----
>>> From: Xiaowei Bao
>>> Sent: 2018年10月26日 17:19
>>> To: 'Kishon Vijay Abraham I' <[email protected]>; [email protected];
>>> [email protected]; [email protected]; [email protected]; Leo Li
>>> <[email protected]>; [email protected]; [email protected];
>>> [email protected]; M.h. Lian <[email protected]>;
>>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Cc: Jiafei Pan <[email protected]>
>>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Kishon Vijay Abraham I <[email protected]>
>>> Sent: 2018年10月26日 13:29
>>> To: Xiaowei Bao <[email protected]>; [email protected];
>>> [email protected]; [email protected]; [email protected]; Leo Li
>>> <[email protected]>; [email protected]; [email protected];
>>> [email protected]; M.h. Lian <[email protected]>;
>>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>>
>>> Hi,
>>>
>>> On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
>>>> Add the PCIe EP mode support for layerscape platform.
>>>>
>>>> Signed-off-by: Xiaowei Bao <[email protected]>
>>>> ---
>>>> drivers/pci/controller/dwc/Makefile | 2 +-
>>>> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161
>>>> ++++++++++++++++++++++++
>>>> 2 files changed, 162 insertions(+), 1 deletions(-) create mode
>>>> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/Makefile
>>>> b/drivers/pci/controller/dwc/Makefile
>>>> index 5d2ce72..b26d617 100644
>>>> --- a/drivers/pci/controller/dwc/Makefile
>>>> +++ b/drivers/pci/controller/dwc/Makefile
>>>> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>>> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>>>> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>>>> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>>>> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>>> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>>> +pci-layerscape-ep.o
>>>> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>>>> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>>>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
>>>> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>> new file mode 100644
>>>> index 0000000..3b33bbc
>>>> --- /dev/null
>>>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>> @@ -0,0 +1,161 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * PCIe controller EP driver for Freescale Layerscape SoCs
>>>> + *
>>>> + * Copyright (C) 2018 NXP Semiconductor.
>>>> + *
>>>> + * Author: Xiaowei Bao <[email protected]> */
>>>> +
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/of_pci.h>
>>>> +#include <linux/of_platform.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/pci.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/resource.h>
>>>> +
>>>> +#include "pcie-designware.h"
>>>> +
>>>> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
>>>
>>> The base address should come from dt.
>>>> +
>>>> +struct ls_pcie_ep {
>>>> + struct dw_pcie *pci;
>>>> +};
>>>> +
>>>> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
>>>> +
>>>> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie) {
>>>> + struct dw_pcie *pci = pcie->pci;
>>>> + u32 header_type;
>>>> +
>>>> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
>>>> + header_type &= 0x7f;
>>>> +
>>>> + return header_type == PCI_HEADER_TYPE_BRIDGE; }
>>>> +
>>>> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
>>>> + return 0;
>>>> +}
>>>
>>> There should be some way by which EP should tell RC that it is not configured yet. Are there no bits to control LTSSM state initialization or Configuration retry status enabling?
>>> [Xiaowei Bao] There have not bits to control LTSSM state to tell the RC it is configured. The start link is auto completed.
>>> [Xiaowei Bao] Hi Kishon, is there any advice?
>>
>> If there is no HW support, I don't think anything could be done here. This could result in RC reading configuration space even before EP is fully initialized.
>> [Xiaowei Bao] The bootloader have initialized the EP device and set the config ready bit, and the kernel don't need to do anything.
>
> What does bootloader initalize? The EP driver here will reinitialize everything right?
> [Xiaowei Bao] The bootloader initialize BAR size, outbound window, inbound window and set the config ready bit.

How will bootloader know the BAR size? The BAR size is based on the function
driver. How does bootloader configure the OB window? The OB window is
configured based on the size and address given by the host dynamically at runtime.

Thanks
Kishon

2018-11-06 06:50:47

by Bao Xiaowei

[permalink] [raw]
Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Hi Kishon,

-----Original Message-----
From: Kishon Vijay Abraham I <[email protected]>
Sent: 2018年11月6日 14:07
To: Xiaowei Bao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Jiafei Pan <[email protected]>
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

(Removed Niklas as mails to him is bouncing)

Hi,

Please fix your email client. Refer Documentation/process/email-clients.rst

On 05/11/18 2:45 PM, Xiaowei Bao wrote:
>
>
> -----Original Message-----
> From: Kishon Vijay Abraham I <[email protected]>
> Sent: 2018年11月5日 16:57
> To: Xiaowei Bao <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; [email protected];
> [email protected]; M.h. Lian <[email protected]>; Mingkai
> Hu <[email protected]>; Roy Zang <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Jiafei Pan <[email protected]>
> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>
> Hi,
>
> On 31/10/18 4:08 PM, Xiaowei Bao wrote:
>>
>>
>> -----Original Message-----
>> From: Kishon Vijay Abraham I <[email protected]>
>> Sent: 2018年10月31日 12:15
>> To: Xiaowei Bao <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected]; Leo Li
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; M.h. Lian <[email protected]>;
>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Cc: Jiafei Pan <[email protected]>
>> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>
>> Hi,
>>
>> On 31/10/18 8:03 AM, Xiaowei Bao wrote:
>>>
>>>
>>> -----Original Message-----
>>> From: Xiaowei Bao
>>> Sent: 2018年10月26日 17:19
>>> To: 'Kishon Vijay Abraham I' <[email protected]>; [email protected];
>>> [email protected]; [email protected]; [email protected]; Leo
>>> robh+Li
>>> <[email protected]>; [email protected]; [email protected];
>>> [email protected]; M.h. Lian <[email protected]>;
>>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Cc: Jiafei Pan <[email protected]>
>>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Kishon Vijay Abraham I <[email protected]>
>>> Sent: 2018年10月26日 13:29
>>> To: Xiaowei Bao <[email protected]>; [email protected];
>>> [email protected]; [email protected]; [email protected]; Leo
>>> robh+Li
>>> <[email protected]>; [email protected]; [email protected];
>>> [email protected]; M.h. Lian <[email protected]>;
>>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>>
>>> Hi,
>>>
>>> On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
>>>> Add the PCIe EP mode support for layerscape platform.
>>>>
>>>> Signed-off-by: Xiaowei Bao <[email protected]>
>>>> ---
>>>> drivers/pci/controller/dwc/Makefile | 2 +-
>>>> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161
>>>> ++++++++++++++++++++++++
>>>> 2 files changed, 162 insertions(+), 1 deletions(-) create mode
>>>> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/Makefile
>>>> b/drivers/pci/controller/dwc/Makefile
>>>> index 5d2ce72..b26d617 100644
>>>> --- a/drivers/pci/controller/dwc/Makefile
>>>> +++ b/drivers/pci/controller/dwc/Makefile
>>>> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>>> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>>>> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>>>> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>>>> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>>> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>>> +pci-layerscape-ep.o
>>>> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>>>> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>>>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
>>>> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>> new file mode 100644
>>>> index 0000000..3b33bbc
>>>> --- /dev/null
>>>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>> @@ -0,0 +1,161 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * PCIe controller EP driver for Freescale Layerscape SoCs
>>>> + *
>>>> + * Copyright (C) 2018 NXP Semiconductor.
>>>> + *
>>>> + * Author: Xiaowei Bao <[email protected]> */
>>>> +
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/of_pci.h>
>>>> +#include <linux/of_platform.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/pci.h>
>>>> +#include <linux/platform_device.h> #include <linux/resource.h>
>>>> +
>>>> +#include "pcie-designware.h"
>>>> +
>>>> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
>>>
>>> The base address should come from dt.
>>>> +
>>>> +struct ls_pcie_ep {
>>>> + struct dw_pcie *pci;
>>>> +};
>>>> +
>>>> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
>>>> +
>>>> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie) {
>>>> + struct dw_pcie *pci = pcie->pci;
>>>> + u32 header_type;
>>>> +
>>>> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
>>>> + header_type &= 0x7f;
>>>> +
>>>> + return header_type == PCI_HEADER_TYPE_BRIDGE; }
>>>> +
>>>> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
>>>> + return 0;
>>>> +}
>>>
>>> There should be some way by which EP should tell RC that it is not configured yet. Are there no bits to control LTSSM state initialization or Configuration retry status enabling?
>>> [Xiaowei Bao] There have not bits to control LTSSM state to tell the RC it is configured. The start link is auto completed.
>>> [Xiaowei Bao] Hi Kishon, is there any advice?
>>
>> If there is no HW support, I don't think anything could be done here. This could result in RC reading configuration space even before EP is fully initialized.
>> [Xiaowei Bao] The bootloader have initialized the EP device and set the config ready bit, and the kernel don't need to do anything.
>
> What does bootloader initalize? The EP driver here will reinitialize everything right?
> [Xiaowei Bao] The bootloader initialize BAR size, outbound window, inbound window and set the config ready bit.

How will bootloader know the BAR size? The BAR size is based on the function driver. How does bootloader configure the OB window? The OB window is configured based on the size and address given by the host dynamically at runtime.

Thanks
Kishon
[Xiaowei Bao] We use the bootloader is u-boot, we implement the EP base feature in the u-boot, e.g.(MEM space access(RC<=>EP), CONFIG space access(RC=>EP)), we set the EP BAR size like kernel(set the PCI_BASE_ADDRESS_n register, but different size), and the outbound windows also be set, and the bar size will be cover by the EP framework driver when kernel start up.
I have to set the dw_pcie_ops, if don't config the dw_pcie_ops there will have call trace, because the DW driver will call the write_dbi or read_dbi function directly but not check the dw_pcie_ops whether is null. I refer to the pcie-designware-plat.c to implement it. I don't know what you said about this, could you tell me the effect, Thanks a lot.

Thanks
Xiaowei

2018-11-09 02:51:44

by Bao Xiaowei

[permalink] [raw]
Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.



-----Original Message-----
From: Xiaowei Bao
Sent: 2018年11月6日 14:48
To: 'Kishon Vijay Abraham I' <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Jiafei Pan <[email protected]>
Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.

Hi Kishon,

-----Original Message-----
From: Kishon Vijay Abraham I <[email protected]>
Sent: 2018年11月6日 14:07
To: Xiaowei Bao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Leo Li <[email protected]>; [email protected]; [email protected]; [email protected]; M.h. Lian <[email protected]>; Mingkai Hu <[email protected]>; Roy Zang <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Jiafei Pan <[email protected]>
Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.

(Removed Niklas as mails to him is bouncing)

Hi,

Please fix your email client. Refer Documentation/process/email-clients.rst

On 05/11/18 2:45 PM, Xiaowei Bao wrote:
>
>
> -----Original Message-----
> From: Kishon Vijay Abraham I <[email protected]>
> Sent: 2018年11月5日 16:57
> To: Xiaowei Bao <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; Leo Li
> <[email protected]>; [email protected]; [email protected];
> [email protected]; M.h. Lian <[email protected]>; Mingkai
> Hu <[email protected]>; Roy Zang <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Jiafei Pan <[email protected]>
> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>
> Hi,
>
> On 31/10/18 4:08 PM, Xiaowei Bao wrote:
>>
>>
>> -----Original Message-----
>> From: Kishon Vijay Abraham I <[email protected]>
>> Sent: 2018年10月31日 12:15
>> To: Xiaowei Bao <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected]; Leo Li
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; M.h. Lian <[email protected]>;
>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Cc: Jiafei Pan <[email protected]>
>> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>
>> Hi,
>>
>> On 31/10/18 8:03 AM, Xiaowei Bao wrote:
>>>
>>>
>>> -----Original Message-----
>>> From: Xiaowei Bao
>>> Sent: 2018年10月26日 17:19
>>> To: 'Kishon Vijay Abraham I' <[email protected]>; [email protected];
>>> [email protected]; [email protected]; [email protected]; Leo
>>> robh+Li
>>> <[email protected]>; [email protected]; [email protected];
>>> [email protected]; M.h. Lian <[email protected]>;
>>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Cc: Jiafei Pan <[email protected]>
>>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Kishon Vijay Abraham I <[email protected]>
>>> Sent: 2018年10月26日 13:29
>>> To: Xiaowei Bao <[email protected]>; [email protected];
>>> [email protected]; [email protected]; [email protected]; Leo
>>> robh+Li
>>> <[email protected]>; [email protected]; [email protected];
>>> [email protected]; M.h. Lian <[email protected]>;
>>> Mingkai Hu <[email protected]>; Roy Zang <[email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>>
>>> Hi,
>>>
>>> On Thursday 25 October 2018 04:39 PM, Xiaowei Bao wrote:
>>>> Add the PCIe EP mode support for layerscape platform.
>>>>
>>>> Signed-off-by: Xiaowei Bao <[email protected]>
>>>> ---
>>>> drivers/pci/controller/dwc/Makefile | 2 +-
>>>> drivers/pci/controller/dwc/pci-layerscape-ep.c | 161
>>>> ++++++++++++++++++++++++
>>>> 2 files changed, 162 insertions(+), 1 deletions(-) create mode
>>>> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/Makefile
>>>> b/drivers/pci/controller/dwc/Makefile
>>>> index 5d2ce72..b26d617 100644
>>>> --- a/drivers/pci/controller/dwc/Makefile
>>>> +++ b/drivers/pci/controller/dwc/Makefile
>>>> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>>> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>>>> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>>>> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>>>> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>>> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>>> +pci-layerscape-ep.o
>>>> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>>>> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>>>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
>>>> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>> new file mode 100644
>>>> index 0000000..3b33bbc
>>>> --- /dev/null
>>>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
>>>> @@ -0,0 +1,161 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * PCIe controller EP driver for Freescale Layerscape SoCs
>>>> + *
>>>> + * Copyright (C) 2018 NXP Semiconductor.
>>>> + *
>>>> + * Author: Xiaowei Bao <[email protected]> */
>>>> +
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/of_pci.h>
>>>> +#include <linux/of_platform.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/pci.h>
>>>> +#include <linux/platform_device.h> #include <linux/resource.h>
>>>> +
>>>> +#include "pcie-designware.h"
>>>> +
>>>> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
>>>
>>> The base address should come from dt.
>>>> +
>>>> +struct ls_pcie_ep {
>>>> + struct dw_pcie *pci;
>>>> +};
>>>> +
>>>> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
>>>> +
>>>> +static bool ls_pcie_is_bridge(struct ls_pcie_ep *pcie) {
>>>> + struct dw_pcie *pci = pcie->pci;
>>>> + u32 header_type;
>>>> +
>>>> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
>>>> + header_type &= 0x7f;
>>>> +
>>>> + return header_type == PCI_HEADER_TYPE_BRIDGE; }
>>>> +
>>>> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
>>>> + return 0;
>>>> +}
>>>
>>> There should be some way by which EP should tell RC that it is not configured yet. Are there no bits to control LTSSM state initialization or Configuration retry status enabling?
>>> [Xiaowei Bao] There have not bits to control LTSSM state to tell the RC it is configured. The start link is auto completed.
>>> [Xiaowei Bao] Hi Kishon, is there any advice?
>>
>> If there is no HW support, I don't think anything could be done here. This could result in RC reading configuration space even before EP is fully initialized.
>> [Xiaowei Bao] The bootloader have initialized the EP device and set the config ready bit, and the kernel don't need to do anything.
>
> What does bootloader initalize? The EP driver here will reinitialize everything right?
> [Xiaowei Bao] The bootloader initialize BAR size, outbound window, inbound window and set the config ready bit.

How will bootloader know the BAR size? The BAR size is based on the function driver. How does bootloader configure the OB window? The OB window is configured based on the size and address given by the host dynamically at runtime.

Thanks
Kishon
[Xiaowei Bao] We use the bootloader is u-boot, we implement the EP base feature in the u-boot, e.g.(MEM space access(RC<=>EP), CONFIG space access(RC=>EP)), we set the EP BAR size like kernel(set the PCI_BASE_ADDRESS_n register, but different size), and the outbound windows also be set, and the bar size will be cover by the EP framework driver when kernel start up.
I have to set the dw_pcie_ops, if don't config the dw_pcie_ops there will have call trace, because the DW driver will call the write_dbi or read_dbi function directly but not check the dw_pcie_ops whether is null. I refer to the pcie-designware-plat.c to implement it. I don't know what you said about this, could you tell me the effect, Thanks a lot.

Thanks
Xiaowei
[Xiaowei Bao] Hi Kishon, is there any advice? Thanks.