From: Bjorn Helgaas <helgaas@kernel.org>
To: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: linux-mips@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
Rob Herring <robh+dt@kernel.org>,
Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
Huacai Chen <chenhc@lemote.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Paul Burton <paulburton@kernel.org>,
linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 2/5] PCI: Add Loongson PCI Controller support
Date: Mon, 4 May 2020 18:43:29 -0500 [thread overview]
Message-ID: <20200504234329.GA300770@bjorn-Precision-5520> (raw)
In-Reply-To: <20200428011429.1852081-3-jiaxun.yang@flygoat.com>
On Tue, Apr 28, 2020 at 09:14:17AM +0800, Jiaxun Yang wrote:
> This controller can be found on Loongson-2K SoC, Loongson-3
> systems with RS780E/LS7A PCH.
>
> The RS780E part of code was previously located at
> arch/mips/pci/ops-loongson3.c and now it can use generic PCI
> driver implementation.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>
> --
> v2:
> - Clean up according to rob's suggestions
> - Claim that it can't work as a module
> v3:
> - Fix a typo
> v4:
> - More clean-ups: Drop flag check, use devfn
> v7:
> - Fix ordering according to huacai's suggestion
> ---
> drivers/pci/controller/Kconfig | 10 +
> drivers/pci/controller/Makefile | 1 +
> drivers/pci/controller/pci-loongson.c | 251 ++++++++++++++++++++++++++
> 3 files changed, 262 insertions(+)
> create mode 100644 drivers/pci/controller/pci-loongson.c
>
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 91bfdb784829..ae36edb1d7db 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -258,6 +258,16 @@ config PCI_HYPERV_INTERFACE
> The Hyper-V PCI Interface is a helper driver allows other drivers to
> have a common interface with the Hyper-V PCI frontend driver.
>
> +config PCI_LOONGSON
> + bool "LOONGSON PCI Controller"
> + depends on MACH_LOONGSON64 || COMPILE_TEST
> + depends on OF
> + depends on PCI_QUIRKS
> + default MACH_LOONGSON64
> + help
> + Say Y here if you want to enable PCI controller support on
> + Loongson systems.
> +
> source "drivers/pci/controller/dwc/Kconfig"
> source "drivers/pci/controller/mobiveil/Kconfig"
> source "drivers/pci/controller/cadence/Kconfig"
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index 158c59771824..fbac4b0190a0 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
> obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
> obj-$(CONFIG_VMD) += vmd.o
> obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
> +obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
This is apparently a PCIe controller, not a Conventional PCI
controller, since you reference PCIe-specific things like MRRS below.
If that's the case, I'd name it pcie-loongson.c.
> # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
> obj-y += dwc/
> obj-y += mobiveil/
> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> new file mode 100644
> index 000000000000..db1ebf42faf3
> --- /dev/null
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -0,0 +1,251 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Loongson PCI Host Controller Driver
> + *
> + * Copyright (C) 2020 Jiaxun Yang <jiaxun.yang@flygoat.com>
> + */
> +
> +#include <linux/of_device.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +
> +#include "../pci.h"
> +
> +/* Device IDs */
> +#define DEV_PCIE_PORT_0 0x7a09
> +#define DEV_PCIE_PORT_1 0x7a19
> +#define DEV_PCIE_PORT_2 0x7a29
> +
> +#define DEV_LS2K_APB 0x7a02
> +#define DEV_LS7A_CONF 0x7a10
> +#define DEV_LS7A_LPC 0x7a0c
> +
> +#define FLAG_CFG0 BIT(0)
> +#define FLAG_CFG1 BIT(1)
> +#define FLAG_DEV_FIX BIT(2)
> +
> +struct loongson_pci {
> + void __iomem *cfg0_base;
> + void __iomem *cfg1_base;
> + struct platform_device *pdev;
> + u32 flags;
> +};
> +
> +/* Fixup wrong class code in PCIe bridges */
> +static void bridge_class_quirk(struct pci_dev *dev)
> +{
> + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_0, bridge_class_quirk);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_1, bridge_class_quirk);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_2, bridge_class_quirk);
> +
> +static void system_bus_quirk(struct pci_dev *pdev)
> +{
> + u16 tmp;
> +
> + pdev->mmio_always_on = 1;
> + pdev->non_compliant_bars = 1;
> + /* Enable MEM & IO Decoding */
> + pci_read_config_word(pdev, PCI_STATUS, &tmp);
> + tmp |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY;
> + pci_write_config_word(pdev, PCI_STATUS, tmp);
This needs some sort of explanation. Is the non_compliant_bars part
an erratum?
And why can't we disable MEM/IO decoding?
You're also writing PCI_COMMAND_* bits to the PCI_STATUS register,
which is clearly broken.
Since we need an update for at least the PCI_STATUS part, there's
a little minor bike-shedding below.
> +}
> +
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS2K_APB, system_bus_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS7A_CONF, system_bus_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS7A_LPC, system_bus_quirk);
> +
> +static void loongson_mrrs_quirk(struct pci_dev *dev)
> +{
> + struct pci_bus *bus = dev->bus;
> + struct pci_dev *bridge;
> + static const struct pci_device_id bridge_devids[] = {
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> + { 0, },
> + };
> +
> +
> + /* look for the matching bridge */
> + while (!pci_is_root_bus(bus)) {
> + bridge = bus->self;
> + bus = bus->parent;
> + /*
> + * Some Loongson PCIE ports has a h/w limitation of
> + * 256 bytes maximum read request size. It can't handle
> + * anything higher than this. So force this limit on
> + * any devices attached under these ports.
s/PCIE/PCIe/
s/ports has/ports have/
s/It can't/They can't/
s/anything higher/anything larger/
> + */
> + if (pci_match_id(bridge_devids, bridge)) {
> + if (pcie_get_readrq(dev) > 256) {
> + dev_info(&dev->dev, "limiting MRRS to 256\n");
pci_info(dev, ...)
> + pcie_set_readrq(dev, 256);
> + }
> + break;
> + }
> + }
> +}
> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> +
> +static void __iomem *cfg1_map(struct loongson_pci *priv, int bus,
> + unsigned int devfn, int where)
> +{
> + unsigned long addroff = 0x0;
> +
> + if (bus != 0)
> + addroff |= BIT(28); /* Type 1 Access */
> + addroff |= (where & 0xff) | ((where & 0xf00) << 16);
> + addroff |= (bus << 16) | (devfn << 8);
> + return priv->cfg1_base + addroff;
> +}
> +
> +static void __iomem *cfg0_map(struct loongson_pci *priv, int bus,
> + unsigned int devfn, int where)
> +{
> + unsigned long addroff = 0x0;
> +
> + if (bus != 0)
> + addroff |= BIT(24); /* Type 1 Access */
> + addroff |= (bus << 16) | (devfn << 8) | where;
> + return priv->cfg0_base + addroff;
> +}
> +
> +void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devfn,
> + int where)
> +{
> + unsigned char busnum = bus->number;
> + struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
> + struct loongson_pci *priv = pci_host_bridge_priv(bridge);
> +
> + /*
> + * Do not read more than one device on the bus other than
> + * the host bridge.
> + */
> + if (bus->primary != 0 && PCI_SLOT(devfn) > 0 &&
> + priv->flags & FLAG_DEV_FIX)
IIUC, FLAG_DEV_FIX is a workaround for a hardware issue, so this would
be easier to read as:
if (priv->flags & FLAG_DEV_FIX && bus->primary != 0 ...
> + return NULL;
> +
> + /* CFG0 can only access standard space */
> + if (where < PCI_CFG_SPACE_SIZE && priv->cfg0_base)
> + return cfg0_map(priv, busnum, devfn, where);
> +
> + /* CFG1 can access extended space */
> + if (where < PCI_CFG_SPACE_EXP_SIZE && priv->cfg1_base)
> + return cfg1_map(priv, busnum, devfn, where);
> +
> + return NULL;
> +}
> +
> +static int loongson_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> +{
> + int irq;
> + u8 val;
> +
> + irq = of_irq_parse_and_map_pci(dev, slot, pin);
> + if (irq > 0)
> + return irq;
> +
> + /* Care i8259 legacy systems */
> + pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &val);
> + /* i8259 only have 15 IRQs */
> + if (val > 15)
> + return 0;
> +
> + return val;
> +}
> +
> +/* H/w only accept 32-bit PCI operations */
I'm sure you're aware that the spec actually requires 8- and 16-bit
operations, at least for config writes. You probably see a few of the
warnings from pci_generic_config_write32()? Just FYI for any future
hardware designs.
> +static struct pci_ops loongson_pci_ops = {
> + .map_bus = pci_loongson_map_bus,
> + .read = pci_generic_config_read32,
> + .write = pci_generic_config_write32,
> +};
> +
> +static const struct of_device_id loongson_pci_of_match[] = {
> + { .compatible = "loongson,ls2k-pci",
> + .data = (void *)(FLAG_CFG0 | FLAG_CFG1 | FLAG_DEV_FIX), },
> + { .compatible = "loongson,ls7a-pci",
> + .data = (void *)(FLAG_CFG0 | FLAG_CFG1 | FLAG_DEV_FIX), },
> + { .compatible = "loongson,rs780e-pci",
> + .data = (void *)(FLAG_CFG0), },
> + {}
> +};
> +
> +static int loongson_pci_probe(struct platform_device *pdev)
> +{
> + struct loongson_pci *priv;
> + struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> + struct pci_host_bridge *bridge;
> + struct resource *regs;
> + int err;
> +
> + if (!node)
> + return -ENODEV;
> +
> + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*priv));
> + if (!bridge)
> + return -ENODEV;
> +
> + priv = pci_host_bridge_priv(bridge);
> + priv->pdev = pdev;
> + priv->flags = (unsigned long)of_device_get_match_data(dev);
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs) {
> + dev_err(dev, "missing mem resources for cfg0\n");
> + return -EINVAL;
> + }
> +
> + priv->cfg0_base = devm_pci_remap_cfg_resource(dev, regs);
> + if (IS_ERR(priv->cfg0_base))
> + return PTR_ERR(priv->cfg0_base);
> +
> + /* CFG1 is optional */
> + if (priv->flags & FLAG_CFG1) {
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!regs)
> + dev_info(dev, "missing mem resource for cfg1\n");
> + else {
> + priv->cfg1_base = devm_pci_remap_cfg_resource(dev, regs);
> + if (IS_ERR(priv->cfg1_base))
> + priv->cfg1_base = NULL;
> + }
> + }
> +
> + err = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
> + &bridge->dma_ranges, NULL);
> + if (err) {
> + dev_err(dev, "Failed to get bridge resources\n");
Some of your messages are capitalized (maybe just this one) and others
are lower-case.
> + return err;
> + }
> +
> + bridge->dev.parent = dev;
> + bridge->sysdata = priv;
> + bridge->ops = &loongson_pci_ops;
> + bridge->map_irq = loongson_map_irq;
> +
> + err = pci_host_probe(bridge);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static struct platform_driver loongson_pci_driver = {
> + .driver = {
> + .name = "loongson-pci",
> + .of_match_table = loongson_pci_of_match,
> + },
> + .probe = loongson_pci_probe,
> +};
> +builtin_platform_driver(loongson_pci_driver);
> --
> 2.26.0.rc2
>
next prev parent reply other threads:[~2020-05-04 23:43 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-27 6:05 [PATCH v6 0/5] Loongson PCI Generic Driver Jiaxun Yang
2020-04-27 6:05 ` [PATCH v6 1/5] PCI: Don't disable decoding when mmio_always_on is set Jiaxun Yang
2020-04-27 7:42 ` Philippe Mathieu-Daudé
2020-04-27 6:05 ` [PATCH v6 2/5] PCI: Add Loongson PCI Controller support Jiaxun Yang
2020-04-27 6:29 ` Huacai Chen
2020-04-27 6:46 ` Jiaxun Yang
2020-04-27 6:05 ` [PATCH v6 3/5] dt-bindings: Document Loongson PCI Host Controller Jiaxun Yang
2020-04-27 6:05 ` [PATCH v6 4/5] MIPS: DTS: Loongson64: Add PCI Controller Node Jiaxun Yang
2020-04-27 6:05 ` [PATCH v6 5/5] MIPS: Loongson64: Switch to generic PCI driver Jiaxun Yang
2020-04-28 1:14 ` [PATCH v7 0/5] Loongson PCI Generic Driver Jiaxun Yang
2020-04-28 1:14 ` [PATCH v7 1/5] PCI: Don't disable decoding when mmio_always_on is set Jiaxun Yang
2020-05-08 18:51 ` Bjorn Helgaas
2020-05-20 20:55 ` Rob Herring
2020-04-28 1:14 ` [PATCH v7 2/5] PCI: Add Loongson PCI Controller support Jiaxun Yang
2020-04-29 18:43 ` Rob Herring
2020-05-04 23:43 ` Bjorn Helgaas [this message]
2020-05-06 6:08 ` Jiaxun Yang
2020-04-28 1:14 ` [PATCH v7 3/5] dt-bindings: Document Loongson PCI Host Controller Jiaxun Yang
2020-04-28 1:14 ` [PATCH v7 4/5] MIPS: DTS: Loongson64: Add PCI Controller Node Jiaxun Yang
2020-04-28 1:14 ` [PATCH v7 5/5] MIPS: Loongson64: Switch to generic PCI driver Jiaxun Yang
2020-05-08 11:34 ` [PATCH v8 1/5] PCI: Don't disable decoding when mmio_always_on is set Jiaxun Yang
2020-05-08 11:34 ` [PATCH v8 2/5] PCI: Add Loongson PCI Controller support Jiaxun Yang
2020-05-08 17:17 ` Bjorn Helgaas
2020-05-08 17:28 ` Jiaxun Yang
2020-05-08 19:34 ` Bjorn Helgaas
2020-05-08 11:34 ` [PATCH v8 3/5] dt-bindings: Document Loongson PCI Host Controller Jiaxun Yang
2020-05-08 11:34 ` [PATCH v8 4/5] MIPS: DTS: Loongson64: Add PCI Controller Node Jiaxun Yang
2020-05-08 11:34 ` [PATCH v8 5/5] MIPS: Loongson64: Switch to generic PCI driver Jiaxun Yang
2020-05-08 22:04 ` Bjorn Helgaas
2020-05-12 7:43 ` [PATCH v9 1/5] PCI: Don't disable decoding when mmio_always_on is set Jiaxun Yang
2020-05-12 7:43 ` [PATCH v9 2/5] PCI: Add Loongson PCI Controller support Jiaxun Yang
2020-05-12 18:06 ` Bjorn Helgaas
2020-05-13 1:20 ` Jiaxun Yang
2020-05-13 15:05 ` Bjorn Helgaas
2020-05-12 7:43 ` [PATCH v9 3/5] dt-bindings: Document Loongson PCI Host Controller Jiaxun Yang
2020-05-12 7:43 ` [PATCH v9 4/5] MIPS: DTS: Loongson64: Add PCI Controller Node Jiaxun Yang
2020-05-12 7:43 ` [PATCH v9 5/5] MIPS: Loongson64: Switch to generic PCI driver Jiaxun Yang
2020-05-14 13:16 ` [PATCH v10 1/5] PCI: Don't disable decoding when mmio_always_on is set Jiaxun Yang
2020-05-14 13:16 ` [PATCH v10 2/5] PCI: Add Loongson PCI Controller support Jiaxun Yang
2020-05-20 11:57 ` Jiaxun Yang
2020-05-22 13:10 ` Lorenzo Pieralisi
2020-05-22 13:32 ` Jiaxun Yang
2020-05-22 13:40 ` Lorenzo Pieralisi
2020-05-22 14:27 ` Thomas Bogendoerfer
2020-05-22 14:27 ` Thomas Bogendoerfer
2020-05-26 9:10 ` Lorenzo Pieralisi
2020-05-14 13:16 ` [PATCH v10 3/5] dt-bindings: Document Loongson PCI Host Controller Jiaxun Yang
2020-05-14 13:16 ` [PATCH v10 4/5] MIPS: DTS: Loongson64: Add PCI Controller Node Jiaxun Yang
2020-05-22 14:25 ` Thomas Bogendoerfer
2020-05-14 13:16 ` [PATCH v10 5/5] MIPS: Loongson64: Switch to generic PCI driver Jiaxun Yang
2020-05-22 14:25 ` Thomas Bogendoerfer
2020-05-22 15:22 ` Lorenzo Pieralisi
2020-05-22 22:36 ` Thomas Bogendoerfer
2020-05-26 9:12 ` Lorenzo Pieralisi
2020-05-26 9:14 ` Jiaxun Yang
2020-05-27 11:34 ` Thomas Bogendoerfer
2020-05-16 8:29 ` [PATCH v4 1/6] irqchip: Add Loongson HyperTransport Vector support Jiaxun Yang
2020-05-16 8:29 ` [PATCH v4 2/6] dt-bindings: interrupt-controller: Add Loongson HTVEC Jiaxun Yang
2020-05-25 10:12 ` Marc Zyngier
2020-05-26 9:26 ` Jiaxun Yang
2020-05-26 9:53 ` Marc Zyngier
2020-05-29 3:51 ` Jiaxun Yang
2020-05-28 20:14 ` Rob Herring
2020-05-16 8:29 ` [PATCH v4 3/6] irqchip: Add Loongson PCH PIC controller Jiaxun Yang
2020-05-16 8:29 ` [PATCH v4 4/6] dt-bindings: interrupt-controller: Add Loongson PCH PIC Jiaxun Yang
2020-05-28 20:15 ` Rob Herring
2020-05-16 8:29 ` [PATCH v4 5/6] irqchip: Add Loongson PCH MSI controller Jiaxun Yang
2020-05-16 8:29 ` [PATCH v4 6/6] dt-bindings: interrupt-controller: Add Loongson PCH MSI Jiaxun Yang
2020-05-26 9:21 ` [PATCH v11 0/5] Loongson Generic PCI v11 Jiaxun Yang
2020-05-26 9:21 ` [PATCH v11 1/5] PCI: Don't disable decoding when mmio_always_on is set Jiaxun Yang
2020-05-26 9:21 ` [PATCH v11 2/5] PCI: Add Loongson PCI Controller support Jiaxun Yang
2020-05-26 9:21 ` [PATCH v11 3/5] dt-bindings: Document Loongson PCI Host Controller Jiaxun Yang
2020-05-26 9:21 ` [PATCH v11 4/5] MIPS: DTS: Loongson64: Add PCI Controller Node Jiaxun Yang
2020-05-26 9:21 ` [PATCH v11 5/5] MIPS: Loongson64: Switch to generic PCI driver Jiaxun Yang
2020-05-27 11:35 ` [PATCH v11 0/5] Loongson Generic PCI v11 Thomas Bogendoerfer
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=20200504234329.GA300770@bjorn-Precision-5520 \
--to=helgaas@kernel.org \
--cc=bhelgaas@google.com \
--cc=chenhc@lemote.com \
--cc=devicetree@vger.kernel.org \
--cc=jiaxun.yang@flygoat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=paulburton@kernel.org \
--cc=robh+dt@kernel.org \
--cc=tsbogend@alpha.franken.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).