lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 3 Dec 2018 04:08:03 +0000
From:   Xiaowei Bao <xiaowei.bao@....com>
To:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>
CC:     "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        Leo Li <leoyang.li@....com>, "kishon@...com" <kishon@...com>,
        "arnd@...db.de" <arnd@...db.de>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "M.h. Lian" <minghuan.lian@....com>,
        Mingkai Hu <mingkai.hu@....com>, Roy Zang <roy.zang@....com>,
        "kstewart@...uxfoundation.org" <kstewart@...uxfoundation.org>,
        "cyrille.pitchen@...e-electrons.com" 
        <cyrille.pitchen@...e-electrons.com>,
        "pombredanne@...b.com" <pombredanne@...b.com>,
        "shawn.lin@...k-chips.com" <shawn.lin@...k-chips.com>,
        "niklas.cassel@...s.com" <niklas.cassel@...s.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: RE: [PATCHv2 5/6] pci: layerscape: Add the EP mode support.

Hi Lorenzo,

-----Original Message-----
From: Lorenzo Pieralisi <lorenzo.pieralisi@....com> 
Sent: 2018年12月1日 0:22
To: Xiaowei Bao <xiaowei.bao@....com>
Cc: bhelgaas@...gle.com; robh+dt@...nel.org; mark.rutland@....com; shawnguo@...nel.org; Leo Li <leoyang.li@....com>; kishon@...com; arnd@...db.de; gregkh@...uxfoundation.org; M.h. Lian <minghuan.lian@....com>; Mingkai Hu <mingkai.hu@....com>; Roy Zang <roy.zang@....com>; kstewart@...uxfoundation.org; cyrille.pitchen@...e-electrons.com; pombredanne@...b.com; shawn.lin@...k-chips.com; niklas.cassel@...s.com; linux-pci@...r.kernel.org; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCHv2 5/6] pci: layerscape: Add the EP mode support.

On Mon, Nov 05, 2018 at 04:46:52PM +0800, Xiaowei Bao wrote:
> Add the PCIe EP mode support for layerscape platform.
> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@....com>
> ---
> v2:
>  - remove the EP mode check function.
> 
>  drivers/pci/controller/dwc/Makefile            |    2 +-
>  drivers/pci/controller/dwc/pci-layerscape-ep.c |  147 
> ++++++++++++++++++++++++
>  2 files changed, 148 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..289618b
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe controller EP driver for Freescale Layerscape SoCs
> + *
> + * Copyright (C) 2018 NXP Semiconductor.
> + *
> + * Author: Xiaowei Bao <xiaowei.bao@....com>  */
> +
> +#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;
> +};

I am not really sure why you need an additional struct.
[Xiaowei Bao] thanks a lot for your comments, I defined this structure in order to add NXP's new chip PCIe EP driver in the future, because other platforms may have some errata to be solved. The structure of defining an LX platform will be more flexible. I can use the driver of the DW platform directly. Just need to modify the DTS, but for the future development of all NXP platform EP drivers, thus adding a new file.

> +#define to_ls_pcie_ep(x)	dev_get_drvdata((x)->dev)

Unused.

> +
> +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;

So if it falls through to the default, we log an error but return 0 ? This does not make much sense.

I know you probably copy/pasted code from DWC platform, that code must be fixed too I suppose.

Lorenzo
[Xiaowei Bao] Thanks a lot for your comments, I will send the v3 patch fix it.

> +}
> +
> +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;
> +
> +	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
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ