[5/6] pci: layerscape: Add the EP mode support.

Commit Message

Xiaowei Bao Oct. 25, 2018, 11:09 a.m. UTC

Add the PCIe EP mode support for layerscape platform.

Signed-off-by: Xiaowei Bao <[e-mail]>
---
 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

Comments

Kishon Vijay Abraham I Oct. 26, 2018, 5:29 a.m. UTC | #1

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 <[e-mail]>
> ---
>  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 <[e-mail]>
> + */
> +
> +#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

Xiaowei Bao Oct. 26, 2018, 9:18 a.m. UTC | #2

-----Original Message-----
From: Kishon Vijay Abraham I <[e-mail]> 
Sent: 2018年10月26日 13:29
To: Xiaowei Bao <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
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 <[e-mail]>
> ---
>  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 <[e-mail]>  */
> +
> +#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

Xiaowei Bao Oct. 31, 2018, 2:33 a.m. UTC | #3

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



-----Original Message-----
From: Kishon Vijay Abraham I <[e-mail]>
Sent: 2018年10月26日 13:29
To: Xiaowei Bao <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
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 <[e-mail]>
> ---
>  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 <[e-mail]>  */
> +
> +#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

Kishon Vijay Abraham I Oct. 31, 2018, 4:15 a.m. UTC | #4

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' <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
> Cc: Jiafei Pan <[e-mail]>
> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
> 
> 
> 
> -----Original Message-----
> From: Kishon Vijay Abraham I <[e-mail]>
> Sent: 2018年10月26日 13:29
> To: Xiaowei Bao <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
> 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 <[e-mail]>
>> ---
>>  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 <[e-mail]>  */
>> +
>> +#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

Xiaowei Bao Oct. 31, 2018, 10:38 a.m. UTC | #5

-----Original Message-----
From: Kishon Vijay Abraham I <[e-mail]> 
Sent: 2018年10月31日 12:15
To: Xiaowei Bao <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
Cc: Jiafei Pan <[e-mail]>
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' <[e-mail]>; [e-mail]; 
> [e-mail]; [e-mail]; [e-mail]; Leo Li 
> <[e-mail]>; [e-mail]; [e-mail]; 
> [e-mail]; M.h. Lian <[e-mail]>; Mingkai 
> Hu <[e-mail]>; Roy Zang <[e-mail]>; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]
> Cc: Jiafei Pan <[e-mail]>
> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
> 
> 
> 
> -----Original Message-----
> From: Kishon Vijay Abraham I <[e-mail]>
> Sent: 2018年10月26日 13:29
> To: Xiaowei Bao <[e-mail]>; [e-mail]; 
> [e-mail]; [e-mail]; [e-mail]; Leo Li 
> <[e-mail]>; [e-mail]; [e-mail]; 
> [e-mail]; M.h. Lian <[e-mail]>; Mingkai 
> Hu <[e-mail]>; Roy Zang <[e-mail]>; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]
> 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 <[e-mail]>
>> ---
>>  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 <[e-mail]>  */
>> +
>> +#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

Kishon Vijay Abraham I Nov. 5, 2018, 8:57 a.m. UTC | #6

Hi,

On 31/10/18 4:08 PM, Xiaowei Bao wrote:
> 
> 
> -----Original Message-----
> From: Kishon Vijay Abraham I <[e-mail]> 
> Sent: 2018年10月31日 12:15
> To: Xiaowei Bao <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
> Cc: Jiafei Pan <[e-mail]>
> 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' <[e-mail]>; [e-mail]; 
>> [e-mail]; [e-mail]; [e-mail]; Leo Li 
>> <[e-mail]>; [e-mail]; [e-mail]; 
>> [e-mail]; M.h. Lian <[e-mail]>; Mingkai 
>> Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]
>> Cc: Jiafei Pan <[e-mail]>
>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>
>>
>>
>> -----Original Message-----
>> From: Kishon Vijay Abraham I <[e-mail]>
>> Sent: 2018年10月26日 13:29
>> To: Xiaowei Bao <[e-mail]>; [e-mail]; 
>> [e-mail]; [e-mail]; [e-mail]; Leo Li 
>> <[e-mail]>; [e-mail]; [e-mail]; 
>> [e-mail]; M.h. Lian <[e-mail]>; Mingkai 
>> Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]
>> 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 <[e-mail]>
>>> ---
>>>  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 <[e-mail]>  */
>>> +
>>> +#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
>

Xiaowei Bao Nov. 5, 2018, 9:15 a.m. UTC | #7

-----Original Message-----
From: Kishon Vijay Abraham I <[e-mail]> 
Sent: 2018年11月5日 16:57
To: Xiaowei Bao <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
Cc: Jiafei Pan <[e-mail]>
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 <[e-mail]>
> Sent: 2018年10月31日 12:15
> To: Xiaowei Bao <[e-mail]>; [e-mail]; 
> [e-mail]; [e-mail]; [e-mail]; Leo Li 
> <[e-mail]>; [e-mail]; [e-mail]; 
> [e-mail]; M.h. Lian <[e-mail]>; Mingkai 
> Hu <[e-mail]>; Roy Zang <[e-mail]>; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]
> Cc: Jiafei Pan <[e-mail]>
> 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' <[e-mail]>; [e-mail];
>> [e-mail]; [e-mail]; [e-mail]; Leo Li
>> <[e-mail]>; [e-mail]; [e-mail]; 
>> [e-mail]; M.h. Lian <[e-mail]>; 
>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>> [e-mail]; [e-mail];
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]
>> Cc: Jiafei Pan <[e-mail]>
>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>
>>
>>
>> -----Original Message-----
>> From: Kishon Vijay Abraham I <[e-mail]>
>> Sent: 2018年10月26日 13:29
>> To: Xiaowei Bao <[e-mail]>; [e-mail];
>> [e-mail]; [e-mail]; [e-mail]; Leo Li
>> <[e-mail]>; [e-mail]; [e-mail]; 
>> [e-mail]; M.h. Lian <[e-mail]>; 
>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>> [e-mail]; [e-mail];
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]
>> 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 <[e-mail]>
>>> ---
>>>  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 <[e-mail]>  */
>>> +
>>> +#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
>

Kishon Vijay Abraham I Nov. 6, 2018, 6:06 a.m. UTC | #8

(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 <[e-mail]> 
> Sent: 2018年11月5日 16:57
> To: Xiaowei Bao <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
> Cc: Jiafei Pan <[e-mail]>
> 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 <[e-mail]>
>> Sent: 2018年10月31日 12:15
>> To: Xiaowei Bao <[e-mail]>; [e-mail]; 
>> [e-mail]; [e-mail]; [e-mail]; Leo Li 
>> <[e-mail]>; [e-mail]; [e-mail]; 
>> [e-mail]; M.h. Lian <[e-mail]>; Mingkai 
>> Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]
>> Cc: Jiafei Pan <[e-mail]>
>> 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' <[e-mail]>; [e-mail];
>>> [e-mail]; [e-mail]; [e-mail]; Leo Li
>>> <[e-mail]>; [e-mail]; [e-mail]; 
>>> [e-mail]; M.h. Lian <[e-mail]>; 
>>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>>> [e-mail]; [e-mail];
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]
>>> Cc: Jiafei Pan <[e-mail]>
>>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Kishon Vijay Abraham I <[e-mail]>
>>> Sent: 2018年10月26日 13:29
>>> To: Xiaowei Bao <[e-mail]>; [e-mail];
>>> [e-mail]; [e-mail]; [e-mail]; Leo Li
>>> <[e-mail]>; [e-mail]; [e-mail]; 
>>> [e-mail]; M.h. Lian <[e-mail]>; 
>>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>>> [e-mail]; [e-mail];
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]
>>> 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 <[e-mail]>
>>>> ---
>>>>  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 <[e-mail]>  */
>>>> +
>>>> +#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 Nov. 6, 2018, 6:48 a.m. UTC | #9

Hi Kishon，

-----Original Message-----
From: Kishon Vijay Abraham I <[e-mail]> 
Sent: 2018年11月6日 14:07
To: Xiaowei Bao <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
Cc: Jiafei Pan <[e-mail]>
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 <[e-mail]>
> Sent: 2018年11月5日 16:57
> To: Xiaowei Bao <[e-mail]>; [e-mail]; 
> [e-mail]; [e-mail]; [e-mail]; Leo Li 
> <[e-mail]>; [e-mail]; [e-mail]; 
> [e-mail]; M.h. Lian <[e-mail]>; Mingkai 
> Hu <[e-mail]>; Roy Zang <[e-mail]>; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]
> Cc: Jiafei Pan <[e-mail]>
> 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 <[e-mail]>
>> Sent: 2018年10月31日 12:15
>> To: Xiaowei Bao <[e-mail]>; [e-mail];
>> [e-mail]; [e-mail]; [e-mail]; Leo Li
>> <[e-mail]>; [e-mail]; [e-mail]; 
>> [e-mail]; M.h. Lian <[e-mail]>; 
>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>> [e-mail]; [e-mail];
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]
>> Cc: Jiafei Pan <[e-mail]>
>> 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' <[e-mail]>; [e-mail];
>>> [e-mail]; [e-mail]; [e-mail]; Leo 
>>> robh+Li
>>> <[e-mail]>; [e-mail]; [e-mail]; 
>>> [e-mail]; M.h. Lian <[e-mail]>; 
>>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>>> [e-mail]; [e-mail];
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]
>>> Cc: Jiafei Pan <[e-mail]>
>>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Kishon Vijay Abraham I <[e-mail]>
>>> Sent: 2018年10月26日 13:29
>>> To: Xiaowei Bao <[e-mail]>; [e-mail];
>>> [e-mail]; [e-mail]; [e-mail]; Leo 
>>> robh+Li
>>> <[e-mail]>; [e-mail]; [e-mail]; 
>>> [e-mail]; M.h. Lian <[e-mail]>; 
>>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>>> [e-mail]; [e-mail];
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]
>>> 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 <[e-mail]>
>>>> ---
>>>>  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 <[e-mail]>  */
>>>> +
>>>> +#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 Nov. 9, 2018, 2:50 a.m. UTC | #10

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

Hi Kishon，

-----Original Message-----
From: Kishon Vijay Abraham I <[e-mail]>
Sent: 2018年11月6日 14:07
To: Xiaowei Bao <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Leo Li <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; M.h. Lian <[e-mail]>; Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
Cc: Jiafei Pan <[e-mail]>
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 <[e-mail]>
> Sent: 2018年11月5日 16:57
> To: Xiaowei Bao <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; Leo Li
> <[e-mail]>; [e-mail]; [e-mail]; 
> [e-mail]; M.h. Lian <[e-mail]>; Mingkai 
> Hu <[e-mail]>; Roy Zang <[e-mail]>; 
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]; 
> [e-mail]; [e-mail]
> Cc: Jiafei Pan <[e-mail]>
> 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 <[e-mail]>
>> Sent: 2018年10月31日 12:15
>> To: Xiaowei Bao <[e-mail]>; [e-mail];
>> [e-mail]; [e-mail]; [e-mail]; Leo Li
>> <[e-mail]>; [e-mail]; [e-mail]; 
>> [e-mail]; M.h. Lian <[e-mail]>; 
>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>> [e-mail]; [e-mail];
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]
>> Cc: Jiafei Pan <[e-mail]>
>> 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' <[e-mail]>; [e-mail];
>>> [e-mail]; [e-mail]; [e-mail]; Leo 
>>> robh+Li
>>> <[e-mail]>; [e-mail]; [e-mail]; 
>>> [e-mail]; M.h. Lian <[e-mail]>; 
>>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>>> [e-mail]; [e-mail];
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]
>>> Cc: Jiafei Pan <[e-mail]>
>>> Subject: RE: [PATCH 5/6] pci: layerscape: Add the EP mode support.
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Kishon Vijay Abraham I <[e-mail]>
>>> Sent: 2018年10月26日 13:29
>>> To: Xiaowei Bao <[e-mail]>; [e-mail];
>>> [e-mail]; [e-mail]; [e-mail]; Leo 
>>> robh+Li
>>> <[e-mail]>; [e-mail]; [e-mail]; 
>>> [e-mail]; M.h. Lian <[e-mail]>; 
>>> Mingkai Hu <[e-mail]>; Roy Zang <[e-mail]>; 
>>> [e-mail]; [e-mail];
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]; 
>>> [e-mail]; [e-mail]
>>> 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 <[e-mail]>
>>>> ---
>>>>  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 <[e-mail]>  */
>>>> +
>>>> +#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.

Patch

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 <[e-mail]>
+ */
+
+#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);