[PATCH v2] PCI: layerscape: Add PCIe support for LS1043a and LS2080a
Bjorn Helgaas
helgaas at kernel.org
Wed Oct 7 10:57:25 PDT 2015
Hi Minghuan,
On Thu, Sep 17, 2015 at 05:13:39PM +0800, Minghuan Lian wrote:
> The patch adds PCIe support for LS1043a and LS2080a.
>
> Signed-off-by: Minghuan Lian <Minghuan.Lian at freescale.com>
> ---
> This patch is based on v4.3-rc1 and [PATCH v9 0/6]
> PCI: hisi: Add PCIe host support for HiSilicon SoC Hip05
> patchset from Zhou Wang.
>
> change log
> v2:
> 1. rename ls2085a to ls2080a
> 2. Add ls_pcie_msi_host_init()
>
> drivers/pci/host/Kconfig | 2 +-
> drivers/pci/host/pci-layerscape.c | 227 ++++++++++++++++++++++++++------------
> 2 files changed, 157 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index ae873be..38fe8a8 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -105,7 +105,7 @@ config PCI_XGENE_MSI
>
> config PCI_LAYERSCAPE
> bool "Freescale Layerscape PCIe controller"
> - depends on OF && ARM
> + depends on OF && (ARM || ARM64)
It seems like there are a couple things going on here, and I wonder if
you can split them out into separate patches.
1) Making this work on ARM64 as well as on ARM. This may be of
interest for other DesignWare-based drivers, so if you split this out,
maybe it would be a useful template for converting the other drivers,
too.
2) Adding LS1043a and LS2080a. Obviously, this is only of interest to
this driver, but maybe it could be separated out into a separate
patch?
> select PCIE_DW
> select MFD_SYSCON
> help
> diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> index b2328ea1..cdb8737 100644
> --- a/drivers/pci/host/pci-layerscape.c
> +++ b/drivers/pci/host/pci-layerscape.c
> @@ -11,7 +11,6 @@
> */
>
> #include <linux/kernel.h>
> -#include <linux/delay.h>
> #include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/of_pci.h>
> @@ -32,27 +31,68 @@
> #define LTSSM_STATE_MASK 0x3f
> #define LTSSM_PCIE_L0 0x11 /* L0 state */
>
> -/* Symbol Timer Register and Filter Mask Register 1 */
> -#define PCIE_STRFMR1 0x71c
> +/* PEX Internal Configuration Registers */
> +#define PCIE_STRFMR1 0x71c /* Symbol Timer & Filter Mask Register1 */
> +#define PCIE_DBI_RO_WR_EN 0x8bc /* DBI Read-Only Write Enable Register */
> +
> +/* PEX LUT registers */
> +#define PCIE_LUT_DBG 0x7FC /* PEX LUT Debug Register */
> +
> +struct ls_pcie_drvdata {
> + u32 lut_offset;
> + u32 ltssm_shift;
> + struct pcie_host_ops *ops;
> +};
>
> struct ls_pcie {
> - struct list_head node;
> - struct device *dev;
> - struct pci_bus *bus;
> - void __iomem *dbi;
> - struct regmap *scfg;
> struct pcie_port pp;
> + const struct ls_pcie_drvdata *drvdata;
> + void __iomem *regs;
> + void __iomem *lut;
> + struct regmap *scfg;
> int index;
> - int msi_irq;
> };
>
> #define to_ls_pcie(x) container_of(x, struct ls_pcie, pp)
>
> -static int ls_pcie_link_up(struct pcie_port *pp)
> +static bool ls_pcie_is_bridge(struct ls_pcie *pcie)
> +{
> + u32 header_type;
> +
> + header_type = ioread32(pcie->regs + (PCI_HEADER_TYPE & ~0x3));
> + header_type = (header_type >> (8 * (PCI_HEADER_TYPE & 0x3))) & 0x7f;
> +
> + return header_type == PCI_HEADER_TYPE_BRIDGE;
> +}
> +
> +/* Clean multi-function bit */
> +static void ls_pcie_clean_multifunction(struct ls_pcie *pcie)
This *clears* the multi-function bit. It's not really clear what
it means to "clean" a bit :)
Why do you read/write 32 bits at a time? Is there a problem with 8-
or 16-bit accesses? If 8- or 16-bit accesses don't work, then we have
to worry about the RW1C problem for some registers.
> +{
> + u32 val;
> +
> + val = ioread32(pcie->regs + (PCI_HEADER_TYPE & ~0x3));
> + val &= ~(1 << 23);
> + iowrite32(val, pcie->regs + (PCI_HEADER_TYPE & ~0x3));
> +}
> +
> +/* Fix class value */
> +static void ls_pcie_fix_class(struct ls_pcie *pcie)
> +{
> + u32 val;
> +
> + val = ioread32(pcie->regs + PCI_CLASS_REVISION);
> + val = (val & 0x0000ffff) | (PCI_CLASS_BRIDGE_PCI << 16);
> + iowrite32(val, pcie->regs + PCI_CLASS_REVISION);
> +}
> +
> +static int ls1021_pcie_link_up(struct pcie_port *pp)
> {
> u32 state;
> struct ls_pcie *pcie = to_ls_pcie(pp);
>
> + if (!pcie->scfg)
> + return 0;
> +
> regmap_read(pcie->scfg, SCFG_PEXMSCPORTSR(pcie->index), &state);
> state = (state >> LTSSM_STATE_SHIFT) & LTSSM_STATE_MASK;
>
> @@ -62,110 +102,155 @@ static int ls_pcie_link_up(struct pcie_port *pp)
> return 1;
> }
>
> -static int ls_pcie_establish_link(struct pcie_port *pp)
> +static void ls1021_pcie_host_init(struct pcie_port *pp)
> {
> - unsigned int retries;
> + struct ls_pcie *pcie = to_ls_pcie(pp);
> + u32 val, index[2];
>
> - for (retries = 0; retries < 200; retries++) {
> - if (dw_pcie_link_up(pp))
> - return 0;
> - usleep_range(100, 1000);
> + pcie->scfg = syscon_regmap_lookup_by_phandle(pp->dev->of_node,
> + "fsl,pcie-scfg");
> + if (IS_ERR(pcie->scfg)) {
> + dev_err(pp->dev, "No syscfg phandle specified\n");
> + pcie->scfg = NULL;
> + return;
> + }
> +
> + if (of_property_read_u32_array(pp->dev->of_node,
> + "fsl,pcie-scfg", index, 2)) {
> + pcie->scfg = NULL;
> + return;
> }
> + pcie->index = index[1];
>
> - dev_err(pp->dev, "phy link never came up\n");
> - return -EINVAL;
> + /*
> + * LS1021A Workaround for internal TKT228622
> + * to fix the INTx hang issue
> + */
> + val = ioread32(pcie->regs + PCIE_STRFMR1);
> + val &= 0xffff;
> + iowrite32(val, pcie->regs + PCIE_STRFMR1);
> +}
> +
> +static int ls_pcie_link_up(struct pcie_port *pp)
> +{
> + struct ls_pcie *pcie = to_ls_pcie(pp);
> + u32 state;
> +
> + state = (ioread32(pcie->lut + PCIE_LUT_DBG) >>
> + pcie->drvdata->ltssm_shift) &
> + LTSSM_STATE_MASK;
> +
> + if (state < LTSSM_PCIE_L0)
> + return 0;
> +
> + return 1;
> }
>
> static void ls_pcie_host_init(struct pcie_port *pp)
> {
> struct ls_pcie *pcie = to_ls_pcie(pp);
> - u32 val;
>
> - dw_pcie_setup_rc(pp);
> - ls_pcie_establish_link(pp);
> + iowrite32(1, pcie->regs + PCIE_DBI_RO_WR_EN);
> + ls_pcie_fix_class(pcie);
> + ls_pcie_clean_multifunction(pcie);
> + iowrite32(0, pcie->regs + PCIE_DBI_RO_WR_EN);
> +}
>
> - /*
> - * LS1021A Workaround for internal TKT228622
> - * to fix the INTx hang issue
> - */
> - val = ioread32(pcie->dbi + PCIE_STRFMR1);
> - val &= 0xffff;
> - iowrite32(val, pcie->dbi + PCIE_STRFMR1);
> +static int ls_pcie_msi_host_init(struct pcie_port *pp,
> + struct msi_controller *chip)
> +{
> + struct device_node *msi_node;
> + struct device_node *np = pp->dev->of_node;
> +
> + msi_node = of_parse_phandle(np, "msi-parent", 0);
> + if (!msi_node) {
> + dev_err(pp->dev, "failed to find msi-parent\n");
> + return -EINVAL;
> + }
This doesn't actually *do* anything except complain if "msi-parent" is
missing. I'm not sure that's worth doing: if we need "msi-parent"
somewhere, we should complain there if we don't find it. If we don't
need it, why complain if it's missing?
> +
> + return 0;
> }
>
> +static struct pcie_host_ops ls1021_pcie_host_ops = {
> + .link_up = ls1021_pcie_link_up,
> + .host_init = ls1021_pcie_host_init,
> + .msi_host_init = ls_pcie_msi_host_init,
> +};
> +
> static struct pcie_host_ops ls_pcie_host_ops = {
> .link_up = ls_pcie_link_up,
> .host_init = ls_pcie_host_init,
> + .msi_host_init = ls_pcie_msi_host_init,
> };
>
> -static int ls_add_pcie_port(struct ls_pcie *pcie)
> -{
> - struct pcie_port *pp;
> - int ret;
> +static struct ls_pcie_drvdata ls1021_drvdata = {
> + .ops = &ls1021_pcie_host_ops,
> +};
>
> - pp = &pcie->pp;
> - pp->dev = pcie->dev;
> - pp->dbi_base = pcie->dbi;
> - pp->root_bus_nr = -1;
> - pp->ops = &ls_pcie_host_ops;
> +static struct ls_pcie_drvdata ls1043_drvdata = {
> + .lut_offset = 0x10000,
> + .ltssm_shift = 24,
> + .ops = &ls_pcie_host_ops,
> +};
>
> - ret = dw_pcie_host_init(pp);
> - if (ret) {
> - dev_err(pp->dev, "failed to initialize host\n");
> - return ret;
> - }
> +static struct ls_pcie_drvdata ls2080_drvdata = {
> + .lut_offset = 0x80000,
> + .ltssm_shift = 0,
> + .ops = &ls_pcie_host_ops,
> +};
>
> - return 0;
> -}
> +static const struct of_device_id ls_pcie_of_match[] = {
> + { .compatible = "fsl,ls1021a-pcie", .data = &ls1021_drvdata },
> + { .compatible = "fsl,ls1043a-pcie", .data = &ls1043_drvdata },
> + { .compatible = "fsl,ls2080a-pcie", .data = &ls2080_drvdata },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, ls_pcie_of_match);
>
> static int __init ls_pcie_probe(struct platform_device *pdev)
> {
> + const struct of_device_id *match;
> struct ls_pcie *pcie;
> - struct resource *dbi_base;
> - u32 index[2];
> + struct pcie_port *pp;
> + struct resource *res;
> int ret;
>
> + match = of_match_device(ls_pcie_of_match, &pdev->dev);
> + if (!match)
> + return -ENODEV;
> +
> pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> if (!pcie)
> return -ENOMEM;
>
> - pcie->dev = &pdev->dev;
> -
> - dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> - pcie->dbi = devm_ioremap_resource(&pdev->dev, dbi_base);
> - if (IS_ERR(pcie->dbi)) {
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> + pcie->regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(pcie->regs)) {
> dev_err(&pdev->dev, "missing *regs* space\n");
> - return PTR_ERR(pcie->dbi);
> + return PTR_ERR(pcie->regs);
> }
>
> - pcie->scfg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> - "fsl,pcie-scfg");
> - if (IS_ERR(pcie->scfg)) {
> - dev_err(&pdev->dev, "No syscfg phandle specified\n");
> - return PTR_ERR(pcie->scfg);
> - }
> + pcie->drvdata = match->data;
> + pcie->lut = pcie->regs + pcie->drvdata->lut_offset;
> + pp = &pcie->pp;
> + pp->dev = &pdev->dev;
> + pp->dbi_base = pcie->regs;
> + pp->ops = pcie->drvdata->ops;
>
> - ret = of_property_read_u32_array(pdev->dev.of_node,
> - "fsl,pcie-scfg", index, 2);
> - if (ret)
> - return ret;
> - pcie->index = index[1];
> + if (!ls_pcie_is_bridge(pcie))
> + return -ENODEV;
>
> - ret = ls_add_pcie_port(pcie);
> - if (ret < 0)
> + ret = dw_pcie_host_init(pp);
We have several DesignWare-based drivers (dra7xx, exynos, imx6, ks,
ls, spear13xx), and I'd like to keep their structure as similar as
possible. For example, they all have basically this structure:
X_pcie_probe
X_add_pcie_port
dw_pcie_host_init # pp->ops->host_init callback
X_pcie_host_init
X_pcie_establish_link
This patch removes ls_add_pcie_port() and ls_pcie_establish_link(), so
we're diverging a bit. That makes it harder to see the similarities
across these drivers, which I think is a loss.
> + if (ret) {
> + dev_err(&pdev->dev, "failed to initialize host\n");
> return ret;
> + }
>
> platform_set_drvdata(pdev, pcie);
>
> return 0;
> }
>
> -static const struct of_device_id ls_pcie_of_match[] = {
> - { .compatible = "fsl,ls1021a-pcie" },
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, ls_pcie_of_match);
> -
> static struct platform_driver ls_pcie_driver = {
> .driver = {
> .name = "layerscape-pcie",
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the linux-arm-kernel
mailing list