From: Rob Herring <robh@kernel.org>
To: Hongtao Wu <wuht06@gmail.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Orson Zhai <orsonzhai@gmail.com>,
Baolin Wang <baolin.wang7@gmail.com>,
Chunyan Zhang <zhang.lyra@gmail.com>,
linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Hongtao Wu <billows.wu@unisoc.com>
Subject: Re: [PATCH v3 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller
Date: Tue, 15 Sep 2020 11:31:01 -0600 [thread overview]
Message-ID: <20200915173101.GB2146778@bogus> (raw)
In-Reply-To: <1599644912-29245-3-git-send-email-wuht06@gmail.com>
On Wed, Sep 09, 2020 at 05:48:32PM +0800, Hongtao Wu wrote:
> From: Hongtao Wu <billows.wu@unisoc.com>
>
> This series adds PCIe controller driver for Unisoc SoCs.
> This controller is based on DesignWare PCIe IP.
>
> Signed-off-by: Hongtao Wu <billows.wu@unisoc.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 13 ++
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-sprd.c | 231 +++++++++++++++++++++++++++++++++
> 3 files changed, 245 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pcie-sprd.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 044a376..0553010 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -311,4 +311,17 @@ config PCIE_AL
> required only for DT-based platforms. ACPI platforms with the
> Annapurna Labs PCIe controller don't need to enable this.
>
> +
> +config PCIE_SPRD
> + tristate "Unisoc PCIe controller - Host Mode"
> + depends on ARCH_SPRD || COMPILE_TEST
> + depends on PCI_MSI_IRQ_DOMAIN
> + select PCIE_DW_HOST
> + help
> + Unisoc PCIe controller uses the DesignWare core. It can be configured
> + as an Endpoint (EP) or a Root complex (RC). In order to enable host
> + mode (the controller works as RC), PCIE_SPRD must be selected.
> + Say Y or M here if you want to PCIe RC controller support on Unisoc
> + SoCs.
> +
> endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index a751553..eb546e9 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_PCI_MESON) += pci-meson.o
> obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> +obj-$(CONFIG_PCIE_SPRD) += pcie-sprd.o
>
> # The following drivers are for devices that use the generic ACPI
> # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-sprd.c b/drivers/pci/controller/dwc/pcie-sprd.c
> new file mode 100644
> index 0000000..cec4f34
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-sprd.c
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe host controller driver for Unisoc SoCs
> + *
> + * Copyright (C) 2020 Unisoc, Inc.
> + *
> + * Author: Hongtao Wu <Billows.Wu@unisoc.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#include "pcie-designware.h"
> +
> +#define NUM_OF_ARGS 5
> +
> +struct sprd_pcie {
> + struct dw_pcie pci;
> +};
> +
> +static int sprd_pcie_syscon_setting(struct platform_device *pdev, char *env)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + int i, count, err;
> + u32 type, delay, reg, mask, val, tmp_val;
> + struct of_phandle_args out_args;
> + struct regmap *iomap;
> + struct device *dev = &pdev->dev;
> +
> + if (!of_find_property(np, env, NULL)) {
> + dev_info(dev, "There isn't property %s in dts\n", env);
> + return 0;
> + }
> +
> + count = of_property_count_elems_of_size(np, env,
> + (NUM_OF_ARGS + 1) * sizeof(u32));
> + dev_info(dev, "Property (%s) reg count is %d :\n", env, count);
> +
> + for (i = 0; i < count; i++) {
> + err = of_parse_phandle_with_fixed_args(np, env, NUM_OF_ARGS,
> + i, &out_args);
> + if (err < 0)
> + return err;
> +
> + type = out_args.args[0];
> + delay = out_args.args[1];
> + reg = out_args.args[2];
> + mask = out_args.args[3];
> + val = out_args.args[4];
> +
> + iomap = syscon_node_to_regmap(out_args.np);
> +
> + switch (type) {
> + case 0:
> + regmap_update_bits(iomap, reg, mask, val);
> + break;
> +
> + case 1:
> + regmap_read(iomap, reg, &tmp_val);
> + tmp_val &= (~mask);
> + tmp_val |= (val & mask);
> + regmap_write(iomap, reg, tmp_val);
> + break;
> + default:
> + break;
> + }
> +
> + if (delay)
> + usleep_range(delay, delay + 10);
> +
> + regmap_read(iomap, reg, &tmp_val);
> + dev_dbg(dev,
> + "%2d:reg[0x%8x] mask[0x%8x] val[0x%8x] result[0x%8x]\n",
> + i, reg, mask, val, tmp_val);
> + }
> +
> + return i;
> +}
> +
> +static int sprd_pcie_perst_assert(struct platform_device *pdev)
> +{
> + return sprd_pcie_syscon_setting(pdev, "sprd,pcie-perst-assert");
Not documented. This should probably use the reset binding.
> +}
> +
> +static int sprd_pcie_perst_deassert(struct platform_device *pdev)
> +{
> + return sprd_pcie_syscon_setting(pdev, "sprd,pcie-perst-deassert");
> +}
> +
> +static int sprd_pcie_power_on(struct platform_device *pdev)
> +{
> + int ret;
> + struct device *dev = &pdev->dev;
> +
> + ret = sprd_pcie_syscon_setting(pdev, "sprd,pcie-poweron-syscons");
> + if (ret < 0)
> + dev_err(dev,
> + "failed to set pcie poweroff syscons, return %d\n",
> + ret);
> +
> + sprd_pcie_perst_deassert(pdev);
> +
> + return ret;
> +}
> +
> +static int sprd_pcie_power_off(struct platform_device *pdev)
> +{
> + int ret;
> + struct device *dev = &pdev->dev;
> +
> + sprd_pcie_perst_assert(pdev);
> +
> + ret = sprd_pcie_syscon_setting(pdev, "sprd,pcie-poweroff-syscons");
> + if (ret < 0)
> + dev_err(dev,
> + "failed to set pcie poweroff syscons, return %d\n",
> + ret);
> +
> + return ret;
> +}
> +
> +static int sprd_pcie_host_init(struct pcie_port *pp)
> +{
> + int ret;
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> + dw_pcie_setup_rc(pp);
> + dw_pcie_msi_init(pp);
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret)
> + dev_err(pci->dev, "pcie ep may has not been powered on yet\n");
> +
> + return ret;
> +}
> +
> +static const struct dw_pcie_host_ops sprd_pcie_host_ops = {
> + .host_init = sprd_pcie_host_init,
> +};
> +
> +static int sprd_add_pcie_port(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct sprd_pcie *ctrl = platform_get_drvdata(pdev);
> + struct dw_pcie *pci = &ctrl->pci;
> + struct pcie_port *pp = &pci->pp;
> +
> + pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi");
> + if (IS_ERR(pci->dbi_base)) {
> + dev_err(dev, "failed to get rc dbi base\n");
> + return PTR_ERR(pci->dbi_base);
> + }
> +
> + pp->ops = &sprd_pcie_host_ops;
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
I don't think this check is needed. The DW core won't setup the MSI if
not enabled, so doesn't matter if msi_irq is initialized.
> + pp->msi_irq = platform_get_irq_byname(pdev, "msi");
> + if (pp->msi_irq < 0) {
> + dev_err(dev, "failed to get msi, return %d\n",
> + pp->msi_irq);
No need to print an error, the core does this.
> + return pp->msi_irq;
> + }
> + }
> +
> + return dw_pcie_host_init(pp);
> +}
> +
> +static int sprd_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct sprd_pcie *ctrl;
> + struct dw_pcie *pci;
> + int ret;
> +
> + ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
> + if (!ctrl)
> + return -ENOMEM;
> +
> + pci = &ctrl->pci;
> + pci->dev = dev;
> +
> + platform_set_drvdata(pdev, ctrl);
> +
> + ret = sprd_pcie_power_on(pdev);
> + if (ret < 0) {
> + dev_err(dev, "failed to get pcie poweron syscons, return %d\n",
> + ret);
> + goto err_power_off;
> + }
> +
> + ret = sprd_add_pcie_port(pdev);
> + if (ret) {
> + dev_warn(dev, "failed to initialize RC controller\n");
> + return ret;
> + }
> +
> + return 0;
> +
> +err_power_off:
> + sprd_pcie_power_off(pdev);
> +
> + return ret;
> +}
> +
> +static const struct of_device_id sprd_pcie_of_match[] = {
> + {
> + .compatible = "sprd,pcie-rc",
> + },
> + {},
> +};
> +
> +static struct platform_driver sprd_pcie_driver = {
> + .probe = sprd_pcie_probe,
You need a .remove hook.
> + .driver = {
> + .name = "sprd-pcie",
> + .of_match_table = sprd_pcie_of_match,
> + },
> +};
> +
> +module_platform_driver(sprd_pcie_driver);
> +
> +MODULE_DESCRIPTION("Unisoc PCIe host controller driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
next prev parent reply other threads:[~2020-09-15 17:49 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-09 9:48 [PATCH v3 0/2] PCI: Add new Unisoc PCIe driver Hongtao Wu
2020-09-09 9:48 ` [PATCH v3 1/2] dt-bindings: PCI: sprd: Document Unisoc PCIe RC host controller Hongtao Wu
2020-09-15 17:25 ` Rob Herring
2020-09-18 3:18 ` Hongtao Wu
2020-09-09 9:48 ` [PATCH v3 2/2] PCI: sprd: Add support for Unisoc SoCs' PCIe controller Hongtao Wu
2020-09-15 17:31 ` Rob Herring [this message]
2020-09-18 2:00 ` Hongtao Wu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200915173101.GB2146778@bogus \
--to=robh@kernel.org \
--cc=baolin.wang7@gmail.com \
--cc=billows.wu@unisoc.com \
--cc=devicetree@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=orsonzhai@gmail.com \
--cc=wuht06@gmail.com \
--cc=zhang.lyra@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.