| Message ID | 20210913182550.264165-1-maz@kernel.org |
|---|---|
| Headers | show |
| Series | PCI: Add support for Apple M1 | expand |
> +config PCIE_APPLE_MSI_DOORBELL_ADDR > + hex > + default 0xfffff000 > + depends on PCIE_APPLE > + > config PCIE_APPLE > tristate "Apple PCIe controller" > depends on ARCH_APPLE || COMPILE_TEST > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c > index 1ed7b90f8360..76344223245d 100644 > --- a/drivers/pci/controller/pcie-apple.c > +++ b/drivers/pci/controller/pcie-apple.c > @@ -120,8 +120,10 @@ > * The doorbell address is set to 0xfffff000, which by convention > * matches what MacOS does, and it is possible to use any other > * address (in the bottom 4GB, as the base register is only 32bit). > + * However, it has to be excluded from the the IOVA range, and the > + * DART driver has to know about it. > */ > -#define DOORBELL_ADDR 0xfffff000 > +#define DOORBELL_ADDR CONFIG_PCIE_APPLE_MSI_DOORBELL_ADDR I'm unsure if Kconfig is the right place for this. But if it is, these hunks should be moved earlier in the series (so the deletion gets squashed away of the hardcoded-in-the-C.)
> +static void apple_msi_compose_msg(struct irq_data *data, struct msi_msg *msg) > +{ > + BUILD_BUG_ON(upper_32_bits(DOORBELL_ADDR)); > + > + msg->address_hi = upper_32_bits(DOORBELL_ADDR); > + msg->address_lo = lower_32_bits(DOORBELL_ADDR); > + msg->data = data->hwirq; > +} ... > @@ -269,6 +378,14 @@ static int apple_pcie_port_setup_irq(struct apple_pcie_port *port) > > irq_set_chained_handler_and_data(irq, apple_port_irq_handler, port); > > + /* Configure MSI base address */ > + writel_relaxed(lower_32_bits(DOORBELL_ADDR), port->base + PORT_MSIADDR); > + > + /* Enable MSIs, shared between all ports */ > + writel_relaxed(0, port->base + PORT_MSIBASE); > + writel_relaxed((ilog2(port->pcie->nvecs) << PORT_MSICFG_L2MSINUM_SHIFT) | > + PORT_MSICFG_EN, port->base + PORT_MSICFG); > + > return 0; > } I think the BUILD_BUG_ON makes more sense next to configuring the base address (which only has a 32-bit register, the BUILD_BUG_ON justifies using writel and not writeq), rather than configuring the message (which specifies the full 64-bits).
On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > The Apple PCIe controller doesn't directly feed the endpoint's > Requester ID to the IOMMU (DART), but instead maps RIDs onto > Stream IDs (SIDs). The DART and the PCIe controller must thus > agree on the SIDs that are used for translation (by using > the 'iommu-map' property). > > For this purpose, parse the 'iommu-map' property each time a > device gets added, and use the resulting translation to configure > the PCIe RID-to-SID mapper. Similarily, remove the translation > if/when the device gets removed. > > This is all driven from a bus notifier which gets registered at > probe time. Hopefully this is the only PCI controller driver > in the whole system. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > drivers/pci/controller/pcie-apple.c | 158 +++++++++++++++++++++++++++- > 1 file changed, 156 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/pcie-apple.c > b/drivers/pci/controller/pcie-apple.c > index 76344223245d..68d71eabe708 100644 > --- a/drivers/pci/controller/pcie-apple.c > +++ b/drivers/pci/controller/pcie-apple.c > @@ -23,8 +23,10 @@ > #include <linux/iopoll.h> > #include <linux/irqchip/chained_irq.h> > #include <linux/irqdomain.h> > +#include <linux/list.h> > #include <linux/module.h> > #include <linux/msi.h> > +#include <linux/notifier.h> > #include <linux/of_irq.h> > #include <linux/pci-ecam.h> > > @@ -116,6 +118,8 @@ > #define PORT_TUNSTAT_PERST_ACK_PEND BIT(1) > #define PORT_PREFMEM_ENABLE 0x00994 > > +#define MAX_RID2SID 64 Do these actually have 64 slots? I thought that was only for the Thunderbolt controllers and that these only had 16. I never checked it myself though and it doesn't make much of a difference for now since only four different RIDs will ever be connected anyway. Sven
On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > From: Alyssa Rosenzweig <alyssa@rosenzweig.io> > > Add a minimal driver to bring up the PCIe bus on Apple system-on-chips, > particularly the Apple M1. This driver exposes the internal bus used for > the USB type-A ports, Ethernet, Wi-Fi, and Bluetooth. Bringing up the > radios requires additional drivers beyond what's necessary for PCIe > itself. > > At this stage, nothing is functionnal. > > Co-developed-by: Stan Skowronek <stan@corellium.com> > Signed-off-by: Stan Skowronek <stan@corellium.com> > Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> > Signed-off-by: Marc Zyngier <maz@kernel.org> > Link: https://lore.kernel.org/r/20210816031621.240268-3-alyssa@rosenzweig.io > --- > MAINTAINERS | 7 + > drivers/pci/controller/Kconfig | 12 ++ > drivers/pci/controller/Makefile | 1 + > drivers/pci/controller/pcie-apple.c | 243 ++++++++++++++++++++++++++++ > 4 files changed, 263 insertions(+) > create mode 100644 drivers/pci/controller/pcie-apple.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 813a847e2d64..9905cc48fed9 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1280,6 +1280,13 @@ S: Maintained > F: Documentation/devicetree/bindings/iommu/apple,dart.yaml > F: drivers/iommu/apple-dart.c > > +APPLE PCIE CONTROLLER DRIVER > +M: Alyssa Rosenzweig <alyssa@rosenzweig.io> > +M: Marc Zyngier <maz@kernel.org> > +L: linux-pci@vger.kernel.org > +S: Maintained > +F: drivers/pci/controller/pcie-apple.c > + > APPLE SMC DRIVER > M: Henrik Rydberg <rydberg@bitmath.org> > L: linux-hwmon@vger.kernel.org > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > index 326f7d13024f..814833a8120d 100644 > --- a/drivers/pci/controller/Kconfig > +++ b/drivers/pci/controller/Kconfig > @@ -312,6 +312,18 @@ config PCIE_HISI_ERR > Say Y here if you want error handling support > for the PCIe controller's errors on HiSilicon HIP SoCs > > +config PCIE_APPLE > + tristate "Apple PCIe controller" > + depends on ARCH_APPLE || COMPILE_TEST > + depends on OF > + depends on PCI_MSI_IRQ_DOMAIN > + help > + Say Y here if you want to enable PCIe controller support on Apple > + system-on-chips, like the Apple M1. This is required for the USB > + type-A ports, Ethernet, Wi-Fi, and Bluetooth. > + > + If unsure, say Y if you have an Apple Silicon system. > + > 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 aaf30b3dcc14..f9d40bad932c 100644 > --- a/drivers/pci/controller/Makefile > +++ b/drivers/pci/controller/Makefile > @@ -37,6 +37,7 @@ obj-$(CONFIG_VMD) += vmd.o > obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o > obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o > obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o > +obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o > # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW > obj-y += dwc/ > obj-y += mobiveil/ > diff --git a/drivers/pci/controller/pcie-apple.c > b/drivers/pci/controller/pcie-apple.c > new file mode 100644 > index 000000000000..f3c414950a10 > --- /dev/null > +++ b/drivers/pci/controller/pcie-apple.c > @@ -0,0 +1,243 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * PCIe host bridge driver for Apple system-on-chips. > + * > + * The HW is ECAM compliant, so once the controller is initialized, > + * the driver mostly deals MSI mapping and handling of per-port > + * interrupts (INTx, management and error signals). > + * > + * Initialization requires enabling power and clocks, along with a > + * number of register pokes. > + * > + * Copyright (C) 2021 Alyssa Rosenzweig <alyssa@rosenzweig.io> > + * Copyright (C) 2021 Google LLC > + * Copyright (C) 2021 Corellium LLC > + * Copyright (C) 2021 Mark Kettenis <kettenis@openbsd.org> > + * > + * Author: Alyssa Rosenzweig <alyssa@rosenzweig.io> > + * Author: Marc Zyngier <maz@kernel.org> > + */ > + > [...] > + > +static inline void rmwl(u32 clr, u32 set, void __iomem *addr) > +{ > + writel_relaxed((readl_relaxed(addr) & ~clr) | set, addr); > +} This helper is a bit strange, especially since it's always only used with either clr != 0 or set != 0 but never (clr = 0 and set = 0) afaict. Maybe create two instead for setting and clearing bits? > + > +static int apple_pcie_setup_port(struct apple_pcie *pcie, > + struct device_node *np) > +{ > + struct platform_device *platform = to_platform_device(pcie->dev); > + struct apple_pcie_port *port; > + struct gpio_desc *reset; > + u32 stat, idx; > + int ret; > + > + reset = gpiod_get_from_of_node(np, "reset-gpios", 0, > + GPIOD_OUT_LOW, "#PERST"); > + if (IS_ERR(reset)) > + return PTR_ERR(reset); > + > + port = devm_kzalloc(pcie->dev, sizeof(*port), GFP_KERNEL); > + if (!port) > + return -ENOMEM; > + > + ret = of_property_read_u32_index(np, "reg", 0, &idx); > + if (ret) > + return ret; > + > + /* Use the first reg entry to work out the port index */ > + port->idx = idx >> 11; > + port->pcie = pcie; > + port->np = np; > + > + port->base = devm_platform_ioremap_resource(platform, port->idx + 2); > + if (IS_ERR(port->base)) > + return -ENODEV; > + > + rmwl(0, PORT_APPCLK_EN, port + PORT_APPCLK); > + > + rmwl(0, PORT_PERST_OFF, port->base + PORT_PERST); > + gpiod_set_value(reset, 1); > + > + ret = readl_relaxed_poll_timeout(port->base + PORT_STATUS, stat, > + stat & PORT_STATUS_READY, 100, 250000); > + if (ret < 0) { > + dev_err(pcie->dev, "port %pOF ready wait timeout\n", np); > + return ret; > + } > + > + /* Flush writes and enable the link */ > + dma_wmb(); This is a DMA barrier but there's no DMA you need to order this against here. I think you can just drop it. > + > + writel_relaxed(PORT_LTSSMCTL_START, port->base + PORT_LTSSMCTL); > + > + return 0; > +} > + Sven
On Mon, Sep 13, 2021 at 1:26 PM Marc Zyngier <maz@kernel.org> wrote: > > Just as we now allow an interrupt map to be parsed when part > of an interrupt controller, there is no reason to ignore an > interrupt map that would be part of a pci device node such as > a root port since we already allow interrupt specifiers. > > This allows the device itself to use the interrupt map for > for its own purpose. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > drivers/pci/of.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index d84381ce82b5..443cebb0622e 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -423,7 +423,7 @@ static int devm_of_pci_get_host_bridge_resources(struct device *dev, > */ > static int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq) > { > - struct device_node *dn, *ppnode; > + struct device_node *dn, *ppnode = NULL; > struct pci_dev *ppdev; > __be32 laddr[3]; > u8 pin; > @@ -452,8 +452,14 @@ static int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args * > if (pin == 0) > return -ENODEV; > > + /* Local interrupt-map in the device node? Use it! */ > + if (dn && of_get_property(dn, "interrupt-map", NULL)) { No need to check 'dn' is not NULL. Otherwise, Reviewed-by: Rob Herring <robh@kernel.org> > + pin = pci_swizzle_interrupt_pin(pdev, pin); > + ppnode = dn; > + } > + > /* Now we walk up the PCI tree */ > - for (;;) { > + while (!ppnode) { > /* Get the pci_dev of our parent */ > ppdev = pdev->bus->self; > > -- > 2.30.2 >
> Date: Tue, 14 Sep 2021 10:35:32 +0100 > From: Marc Zyngier <maz@kernel.org> > > On Mon, 13 Sep 2021 21:45:13 +0100, > "Sven Peter" <sven@svenpeter.dev> wrote: > > > > On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > > > The Apple PCIe controller doesn't directly feed the endpoint's > > > Requester ID to the IOMMU (DART), but instead maps RIDs onto > > > Stream IDs (SIDs). The DART and the PCIe controller must thus > > > agree on the SIDs that are used for translation (by using > > > the 'iommu-map' property). > > > > > > For this purpose, parse the 'iommu-map' property each time a > > > device gets added, and use the resulting translation to configure > > > the PCIe RID-to-SID mapper. Similarily, remove the translation > > > if/when the device gets removed. > > > > > > This is all driven from a bus notifier which gets registered at > > > probe time. Hopefully this is the only PCI controller driver > > > in the whole system. > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > --- > > > drivers/pci/controller/pcie-apple.c | 158 +++++++++++++++++++++++++++- > > > 1 file changed, 156 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pci/controller/pcie-apple.c > > > b/drivers/pci/controller/pcie-apple.c > > > index 76344223245d..68d71eabe708 100644 > > > --- a/drivers/pci/controller/pcie-apple.c > > > +++ b/drivers/pci/controller/pcie-apple.c > > > @@ -23,8 +23,10 @@ > > > #include <linux/iopoll.h> > > > #include <linux/irqchip/chained_irq.h> > > > #include <linux/irqdomain.h> > > > +#include <linux/list.h> > > > #include <linux/module.h> > > > #include <linux/msi.h> > > > +#include <linux/notifier.h> > > > #include <linux/of_irq.h> > > > #include <linux/pci-ecam.h> > > > > > > @@ -116,6 +118,8 @@ > > > #define PORT_TUNSTAT_PERST_ACK_PEND BIT(1) > > > #define PORT_PREFMEM_ENABLE 0x00994 > > > > > > +#define MAX_RID2SID 64 > > > > Do these actually have 64 slots? I thought that was only for > > the Thunderbolt controllers and that these only had 16. > > You are indeed right, and I blindly used the limit used in the > Correlium driver. Using entries from 16 onward result in a non booting > system. The registers do not fault though, and simply ignore writes. I > came up with an simple fix for this, see below. Or should be add a property to the DT binding to indicate the number of entries (using a default of 16)? We don't have to add that property right away; we can delay that until we actually try to support the Thunderbolt ports. In case you didn't know already, RIDs that have no mapping in the RID2SID table map to SID 0. That's why I picked 1 as the SID in the iommu-map property for the port. > > I never checked it myself though and it doesn't make much > > of a difference for now since only four different RIDs will > > ever be connected anyway. > > Four? I guess the radios expose more than a single RID? At this point, on the M1 mini there is the Broadcom BCM4378 WiFi/BT device (which has two functions), the Fresco Logic FL1100 xHCI controller (single function) and the Broadcom BCM57765 Ethernet controller. So yes, there are for RIDs. Cheers, Mark > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c > index 68d71eabe708..ec9e7abd2aca 100644 > --- a/drivers/pci/controller/pcie-apple.c > +++ b/drivers/pci/controller/pcie-apple.c > @@ -148,6 +148,7 @@ struct apple_pcie_port { > struct irq_domain *domain; > struct list_head entry; > DECLARE_BITMAP( sid_map, MAX_RID2SID); > + int sid_map_sz; > int idx; > }; > > @@ -495,12 +496,12 @@ static int apple_pcie_setup_refclk(struct apple_pcie *pcie, > return 0; > } > > -static void apple_pcie_rid2sid_write(struct apple_pcie_port *port, > +static u32 apple_pcie_rid2sid_write(struct apple_pcie_port *port, > int idx, u32 val) > { > writel_relaxed(val, port->base + PORT_RID2SID(idx)); > /* Read back to ensure completion of the write */ > - (void)readl_relaxed(port->base + PORT_RID2SID(idx)); > + return readl_relaxed(port->base + PORT_RID2SID(idx)); > } > > static int apple_pcie_setup_port(struct apple_pcie *pcie, > @@ -557,9 +558,16 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie, > if (ret) > return ret; > > - /* Reset all RID/SID mappings */ > - for (i = 0; i < MAX_RID2SID; i++) > + /* Reset all RID/SID mappings, and check for RAZ/WI registers */ > + for (i = 0; i < MAX_RID2SID; i++) { > + if (apple_pcie_rid2sid_write(port, i, 0xbad1d) != 0xbad1d) > + break; > apple_pcie_rid2sid_write(port, i, 0); > + } > + > + dev_dbg(pcie->dev, "%pOF: %d RID/SID mapping entries\n", np, i); > + > + port->sid_map_sz = i; > > list_add_tail(&port->entry, &pcie->ports); > init_completion(&pcie->event); > @@ -667,7 +675,7 @@ static int apple_pcie_add_device(struct pci_dev *pdev) > return err; > > mutex_lock(&port->pcie->lock); > - sid_idx = bitmap_find_free_region(port->sid_map, MAX_RID2SID, 0); > + sid_idx = bitmap_find_free_region(port->sid_map, port->sid_map_sz, 0); > mutex_unlock(&port->pcie->lock); > > if (sid_idx < 0) > @@ -696,7 +704,7 @@ static void apple_pcie_release_device(struct pci_dev *pdev) > > mutex_lock(&port->pcie->lock); > > - for_each_set_bit(idx, port->sid_map, MAX_RID2SID) { > + for_each_set_bit(idx, port->sid_map, port->sid_map_sz) { > u32 val; > > val = readl_relaxed(port->base + PORT_RID2SID(idx)); > > -- > Without deviation from the norm, progress is not possible. >
On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > The MSI doorbell on Apple HW can be any address in the low 4GB > range. However, the MSI write is matched by the PCIe block before > hitting the iommu. It must thus be excluded from the IOVA range > that is assigned to any PCIe device. > > Signed-off-by: Marc Zyngier <maz@kernel.org> It's not pretty but I'm not aware of any better solution and this should work as long as these two are always paired. With the small nit below addressed: Reviewed-by: Sven Peter <sven@svenpeter.dev> > --- > drivers/iommu/apple-dart.c | 25 +++++++++++++++++++++++++ > drivers/pci/controller/Kconfig | 5 +++++ > drivers/pci/controller/pcie-apple.c | 4 +++- > 3 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c > index 559db9259e65..d1456663688e 100644 > --- a/drivers/iommu/apple-dart.c > +++ b/drivers/iommu/apple-dart.c > @@ -721,6 +721,29 @@ static int apple_dart_def_domain_type(struct device *dev) > return 0; > } > > +#define DOORBELL_ADDR (CONFIG_PCIE_APPLE_MSI_DOORBELL_ADDR & PAGE_MASK) > + > +static void apple_dart_get_resv_regions(struct device *dev, > + struct list_head *head) > +{ > +#ifdef CONFIG_PCIE_APPLE I think using IS_ENABLED would be better here in case the pcie driver is built as a module which would then only define CONFIG_PCIE_APPLE_MODULE AIUI. Thanks, Sven
On Tue, Sep 14, 2021, at 11:35, Marc Zyngier wrote: > On Mon, 13 Sep 2021 21:45:13 +0100, > "Sven Peter" <sven@svenpeter.dev> wrote: > > > > > > > > On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > > > The Apple PCIe controller doesn't directly feed the endpoint's > > > Requester ID to the IOMMU (DART), but instead maps RIDs onto > > > Stream IDs (SIDs). The DART and the PCIe controller must thus > > > agree on the SIDs that are used for translation (by using > > > the 'iommu-map' property). > > > > > > For this purpose, parse the 'iommu-map' property each time a > > > device gets added, and use the resulting translation to configure > > > the PCIe RID-to-SID mapper. Similarily, remove the translation > > > if/when the device gets removed. > > > > > > This is all driven from a bus notifier which gets registered at > > > probe time. Hopefully this is the only PCI controller driver > > > in the whole system. > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > --- > > > drivers/pci/controller/pcie-apple.c | 158 +++++++++++++++++++++++++++- > > > 1 file changed, 156 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pci/controller/pcie-apple.c > > > b/drivers/pci/controller/pcie-apple.c > > > index 76344223245d..68d71eabe708 100644 > > > --- a/drivers/pci/controller/pcie-apple.c > > > +++ b/drivers/pci/controller/pcie-apple.c > > > @@ -23,8 +23,10 @@ > > > #include <linux/iopoll.h> > > > #include <linux/irqchip/chained_irq.h> > > > #include <linux/irqdomain.h> > > > +#include <linux/list.h> > > > #include <linux/module.h> > > > #include <linux/msi.h> > > > +#include <linux/notifier.h> > > > #include <linux/of_irq.h> > > > #include <linux/pci-ecam.h> > > > > > > @@ -116,6 +118,8 @@ > > > #define PORT_TUNSTAT_PERST_ACK_PEND BIT(1) > > > #define PORT_PREFMEM_ENABLE 0x00994 > > > > > > +#define MAX_RID2SID 64 > > > > Do these actually have 64 slots? I thought that was only for > > the Thunderbolt controllers and that these only had 16. > > You are indeed right, and I blindly used the limit used in the > Correlium driver. Using entries from 16 onward result in a non booting > system. The registers do not fault though, and simply ignore writes. I > came up with an simple fix for this, see below. Looks good to me and at least I prefer that to an additional property in the device tree. Reviewed-by: Sven Peter <sven@svenpeter.dev> Thanks, Sven
On Mon, 13 Sep 2021 21:43:23 +0100, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote: > > > +static void apple_msi_compose_msg(struct irq_data *data, struct msi_msg *msg) > > +{ > > + BUILD_BUG_ON(upper_32_bits(DOORBELL_ADDR)); > > + > > + msg->address_hi = upper_32_bits(DOORBELL_ADDR); > > + msg->address_lo = lower_32_bits(DOORBELL_ADDR); > > + msg->data = data->hwirq; > > +} > ... > > @@ -269,6 +378,14 @@ static int apple_pcie_port_setup_irq(struct apple_pcie_port *port) > > > > irq_set_chained_handler_and_data(irq, apple_port_irq_handler, port); > > > > + /* Configure MSI base address */ > > + writel_relaxed(lower_32_bits(DOORBELL_ADDR), port->base + PORT_MSIADDR); > > + > > + /* Enable MSIs, shared between all ports */ > > + writel_relaxed(0, port->base + PORT_MSIBASE); > > + writel_relaxed((ilog2(port->pcie->nvecs) << PORT_MSICFG_L2MSINUM_SHIFT) | > > + PORT_MSICFG_EN, port->base + PORT_MSICFG); > > + > > return 0; > > } > > I think the BUILD_BUG_ON makes more sense next to configuring the base > address (which only has a 32-bit register, the BUILD_BUG_ON justifies > using writel and not writeq), rather than configuring the message (which > specifies the full 64-bits). Indeed, this makes a bit more sense. Thanks for pointing this out. M.
On Tue, 14 Sep 2021 10:56:05 +0100, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > Date: Tue, 14 Sep 2021 10:35:32 +0100 > > From: Marc Zyngier <maz@kernel.org> > > > > On Mon, 13 Sep 2021 21:45:13 +0100, > > "Sven Peter" <sven@svenpeter.dev> wrote: > > > > > > On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > > > > The Apple PCIe controller doesn't directly feed the endpoint's > > > > Requester ID to the IOMMU (DART), but instead maps RIDs onto > > > > Stream IDs (SIDs). The DART and the PCIe controller must thus > > > > agree on the SIDs that are used for translation (by using > > > > the 'iommu-map' property). > > > > > > > > For this purpose, parse the 'iommu-map' property each time a > > > > device gets added, and use the resulting translation to configure > > > > the PCIe RID-to-SID mapper. Similarily, remove the translation > > > > if/when the device gets removed. > > > > > > > > This is all driven from a bus notifier which gets registered at > > > > probe time. Hopefully this is the only PCI controller driver > > > > in the whole system. > > > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > > --- > > > > drivers/pci/controller/pcie-apple.c | 158 +++++++++++++++++++++++++++- > > > > 1 file changed, 156 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/pci/controller/pcie-apple.c > > > > b/drivers/pci/controller/pcie-apple.c > > > > index 76344223245d..68d71eabe708 100644 > > > > --- a/drivers/pci/controller/pcie-apple.c > > > > +++ b/drivers/pci/controller/pcie-apple.c > > > > @@ -23,8 +23,10 @@ > > > > #include <linux/iopoll.h> > > > > #include <linux/irqchip/chained_irq.h> > > > > #include <linux/irqdomain.h> > > > > +#include <linux/list.h> > > > > #include <linux/module.h> > > > > #include <linux/msi.h> > > > > +#include <linux/notifier.h> > > > > #include <linux/of_irq.h> > > > > #include <linux/pci-ecam.h> > > > > > > > > @@ -116,6 +118,8 @@ > > > > #define PORT_TUNSTAT_PERST_ACK_PEND BIT(1) > > > > #define PORT_PREFMEM_ENABLE 0x00994 > > > > > > > > +#define MAX_RID2SID 64 > > > > > > Do these actually have 64 slots? I thought that was only for > > > the Thunderbolt controllers and that these only had 16. > > > > You are indeed right, and I blindly used the limit used in the > > Correlium driver. Using entries from 16 onward result in a non booting > > system. The registers do not fault though, and simply ignore writes. I > > came up with an simple fix for this, see below. > > Or should be add a property to the DT binding to indicate the number > of entries (using a default of 16)? We don't have to add that > property right away; we can delay that until we actually try to > support the Thunderbolt ports. I'd rather only add a property for things we cannot discover ourselves. And indeed, we don't have to decide on this right now. > In case you didn't know already, RIDs that have no mapping in the > RID2SID table map to SID 0. That's why I picked 1 as the SID in the > iommu-map property for the port. I sort-off guessed, as using 0 made everything work by 'magic', while using your DT prevented the machine from booting. I tend to dislike magic, hence this patch. > > > > I never checked it myself though and it doesn't make much > > > of a difference for now since only four different RIDs will > > > ever be connected anyway. > > > > Four? I guess the radios expose more than a single RID? > > At this point, on the M1 mini there is the Broadcom BCM4378 WiFi/BT > device (which has two functions), the Fresco Logic FL1100 xHCI > controller (single function) and the Broadcom BCM57765 Ethernet > controller. So yes, there are for RIDs. But as far as I can see, the RID-to-SID mapping is per port. So at most, we have two RIDs per port/DART, not four. Or am I missing something altogether? M.
On Mon, 13 Sep 2021 21:48:47 +0100, "Sven Peter" <sven@svenpeter.dev> wrote: > > > > On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > > From: Alyssa Rosenzweig <alyssa@rosenzweig.io> > > > > Add a minimal driver to bring up the PCIe bus on Apple system-on-chips, > > particularly the Apple M1. This driver exposes the internal bus used for > > the USB type-A ports, Ethernet, Wi-Fi, and Bluetooth. Bringing up the > > radios requires additional drivers beyond what's necessary for PCIe > > itself. > > > > At this stage, nothing is functionnal. > > > > Co-developed-by: Stan Skowronek <stan@corellium.com> > > Signed-off-by: Stan Skowronek <stan@corellium.com> > > Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > Link: https://lore.kernel.org/r/20210816031621.240268-3-alyssa@rosenzweig.io > > --- > > MAINTAINERS | 7 + > > drivers/pci/controller/Kconfig | 12 ++ > > drivers/pci/controller/Makefile | 1 + > > drivers/pci/controller/pcie-apple.c | 243 ++++++++++++++++++++++++++++ > > 4 files changed, 263 insertions(+) > > create mode 100644 drivers/pci/controller/pcie-apple.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 813a847e2d64..9905cc48fed9 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -1280,6 +1280,13 @@ S: Maintained > > F: Documentation/devicetree/bindings/iommu/apple,dart.yaml > > F: drivers/iommu/apple-dart.c > > > > +APPLE PCIE CONTROLLER DRIVER > > +M: Alyssa Rosenzweig <alyssa@rosenzweig.io> > > +M: Marc Zyngier <maz@kernel.org> > > +L: linux-pci@vger.kernel.org > > +S: Maintained > > +F: drivers/pci/controller/pcie-apple.c > > + > > APPLE SMC DRIVER > > M: Henrik Rydberg <rydberg@bitmath.org> > > L: linux-hwmon@vger.kernel.org > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > > index 326f7d13024f..814833a8120d 100644 > > --- a/drivers/pci/controller/Kconfig > > +++ b/drivers/pci/controller/Kconfig > > @@ -312,6 +312,18 @@ config PCIE_HISI_ERR > > Say Y here if you want error handling support > > for the PCIe controller's errors on HiSilicon HIP SoCs > > > > +config PCIE_APPLE > > + tristate "Apple PCIe controller" > > + depends on ARCH_APPLE || COMPILE_TEST > > + depends on OF > > + depends on PCI_MSI_IRQ_DOMAIN > > + help > > + Say Y here if you want to enable PCIe controller support on Apple > > + system-on-chips, like the Apple M1. This is required for the USB > > + type-A ports, Ethernet, Wi-Fi, and Bluetooth. > > + > > + If unsure, say Y if you have an Apple Silicon system. > > + > > 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 aaf30b3dcc14..f9d40bad932c 100644 > > --- a/drivers/pci/controller/Makefile > > +++ b/drivers/pci/controller/Makefile > > @@ -37,6 +37,7 @@ obj-$(CONFIG_VMD) += vmd.o > > obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o > > obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o > > obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o > > +obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o > > # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW > > obj-y += dwc/ > > obj-y += mobiveil/ > > diff --git a/drivers/pci/controller/pcie-apple.c > > b/drivers/pci/controller/pcie-apple.c > > new file mode 100644 > > index 000000000000..f3c414950a10 > > --- /dev/null > > +++ b/drivers/pci/controller/pcie-apple.c > > @@ -0,0 +1,243 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * PCIe host bridge driver for Apple system-on-chips. > > + * > > + * The HW is ECAM compliant, so once the controller is initialized, > > + * the driver mostly deals MSI mapping and handling of per-port > > + * interrupts (INTx, management and error signals). > > + * > > + * Initialization requires enabling power and clocks, along with a > > + * number of register pokes. > > + * > > + * Copyright (C) 2021 Alyssa Rosenzweig <alyssa@rosenzweig.io> > > + * Copyright (C) 2021 Google LLC > > + * Copyright (C) 2021 Corellium LLC > > + * Copyright (C) 2021 Mark Kettenis <kettenis@openbsd.org> > > + * > > + * Author: Alyssa Rosenzweig <alyssa@rosenzweig.io> > > + * Author: Marc Zyngier <maz@kernel.org> > > + */ > > + > > [...] > > + > > +static inline void rmwl(u32 clr, u32 set, void __iomem *addr) > > +{ > > + writel_relaxed((readl_relaxed(addr) & ~clr) | set, addr); > > +} > > This helper is a bit strange, especially since it's always only used > with either clr != 0 or set != 0 but never (clr = 0 and set = 0) afaict. > Maybe create two instead for setting and clearing bits? That's indeed nicer, and it makes the code more readable. > > > + > > +static int apple_pcie_setup_port(struct apple_pcie *pcie, > > + struct device_node *np) > > +{ > > + struct platform_device *platform = to_platform_device(pcie->dev); > > + struct apple_pcie_port *port; > > + struct gpio_desc *reset; > > + u32 stat, idx; > > + int ret; > > + > > + reset = gpiod_get_from_of_node(np, "reset-gpios", 0, > > + GPIOD_OUT_LOW, "#PERST"); > > + if (IS_ERR(reset)) > > + return PTR_ERR(reset); > > + > > + port = devm_kzalloc(pcie->dev, sizeof(*port), GFP_KERNEL); > > + if (!port) > > + return -ENOMEM; > > + > > + ret = of_property_read_u32_index(np, "reg", 0, &idx); > > + if (ret) > > + return ret; > > + > > + /* Use the first reg entry to work out the port index */ > > + port->idx = idx >> 11; > > + port->pcie = pcie; > > + port->np = np; > > + > > + port->base = devm_platform_ioremap_resource(platform, port->idx + 2); > > + if (IS_ERR(port->base)) > > + return -ENODEV; > > + > > + rmwl(0, PORT_APPCLK_EN, port + PORT_APPCLK); > > + > > + rmwl(0, PORT_PERST_OFF, port->base + PORT_PERST); > > + gpiod_set_value(reset, 1); > > + > > + ret = readl_relaxed_poll_timeout(port->base + PORT_STATUS, stat, > > + stat & PORT_STATUS_READY, 100, 250000); > > + if (ret < 0) { > > + dev_err(pcie->dev, "port %pOF ready wait timeout\n", np); > > + return ret; > > + } > > + > > + /* Flush writes and enable the link */ > > + dma_wmb(); > > This is a DMA barrier but there's no DMA you need to order this against > here. I think you can just drop it. Indeed, this is all MMIO based, and doesn't refer to anything in memory. Barrier gone. Thanks, M.
> Date: Fri, 17 Sep 2021 10:19:02 +0100 > From: Marc Zyngier <maz@kernel.org> > > On Tue, 14 Sep 2021 10:56:05 +0100, > Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > > > Date: Tue, 14 Sep 2021 10:35:32 +0100 > > > From: Marc Zyngier <maz@kernel.org> > > > > > > On Mon, 13 Sep 2021 21:45:13 +0100, > > > "Sven Peter" <sven@svenpeter.dev> wrote: > > > > > > > > On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > > > > > The Apple PCIe controller doesn't directly feed the endpoint's > > > > > Requester ID to the IOMMU (DART), but instead maps RIDs onto > > > > > Stream IDs (SIDs). The DART and the PCIe controller must thus > > > > > agree on the SIDs that are used for translation (by using > > > > > the 'iommu-map' property). > > > > > > > > > > For this purpose, parse the 'iommu-map' property each time a > > > > > device gets added, and use the resulting translation to configure > > > > > the PCIe RID-to-SID mapper. Similarily, remove the translation > > > > > if/when the device gets removed. > > > > > > > > > > This is all driven from a bus notifier which gets registered at > > > > > probe time. Hopefully this is the only PCI controller driver > > > > > in the whole system. > > > > > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > > > --- > > > > > drivers/pci/controller/pcie-apple.c | 158 +++++++++++++++++++++++++++- > > > > > 1 file changed, 156 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/pci/controller/pcie-apple.c > > > > > b/drivers/pci/controller/pcie-apple.c > > > > > index 76344223245d..68d71eabe708 100644 > > > > > --- a/drivers/pci/controller/pcie-apple.c > > > > > +++ b/drivers/pci/controller/pcie-apple.c > > > > > @@ -23,8 +23,10 @@ > > > > > #include <linux/iopoll.h> > > > > > #include <linux/irqchip/chained_irq.h> > > > > > #include <linux/irqdomain.h> > > > > > +#include <linux/list.h> > > > > > #include <linux/module.h> > > > > > #include <linux/msi.h> > > > > > +#include <linux/notifier.h> > > > > > #include <linux/of_irq.h> > > > > > #include <linux/pci-ecam.h> > > > > > > > > > > @@ -116,6 +118,8 @@ > > > > > #define PORT_TUNSTAT_PERST_ACK_PEND BIT(1) > > > > > #define PORT_PREFMEM_ENABLE 0x00994 > > > > > > > > > > +#define MAX_RID2SID 64 > > > > > > > > Do these actually have 64 slots? I thought that was only for > > > > the Thunderbolt controllers and that these only had 16. > > > > > > You are indeed right, and I blindly used the limit used in the > > > Correlium driver. Using entries from 16 onward result in a non booting > > > system. The registers do not fault though, and simply ignore writes. I > > > came up with an simple fix for this, see below. > > > > Or should be add a property to the DT binding to indicate the number > > of entries (using a default of 16)? We don't have to add that > > property right away; we can delay that until we actually try to > > support the Thunderbolt ports. > > I'd rather only add a property for things we cannot discover > ourselves. And indeed, we don't have to decide on this right now. > > > In case you didn't know already, RIDs that have no mapping in the > > RID2SID table map to SID 0. That's why I picked 1 as the SID in the > > iommu-map property for the port. > > I sort-off guessed, as using 0 made everything work by 'magic', while > using your DT prevented the machine from booting. I tend to dislike > magic, hence this patch. Right. I deliberately used SID 1 in the DT to make sure other devices on the bus couldn't accidentally use IOMMU mappings for a device that was mapped to SID 0. > > > > I never checked it myself though and it doesn't make much > > > > of a difference for now since only four different RIDs will > > > > ever be connected anyway. > > > > > > Four? I guess the radios expose more than a single RID? > > > > At this point, on the M1 mini there is the Broadcom BCM4378 WiFi/BT > > device (which has two functions), the Fresco Logic FL1100 xHCI > > controller (single function) and the Broadcom BCM57765 Ethernet > > controller. So yes, there are for RIDs. > > But as far as I can see, the RID-to-SID mapping is per port. So at > most, we have two RIDs per port/DART, not four. Or am I missing > something altogether? No you're not missing anything.
On Tue, 14 Sep 2021 14:54:07 +0100, "Sven Peter" <sven@svenpeter.dev> wrote: > > > > On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > > The MSI doorbell on Apple HW can be any address in the low 4GB > > range. However, the MSI write is matched by the PCIe block before > > hitting the iommu. It must thus be excluded from the IOVA range > > that is assigned to any PCIe device. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > It's not pretty but I'm not aware of any better solution and this should > work as long as these two are always paired. With the small nit below > addressed: > > Reviewed-by: Sven Peter <sven@svenpeter.dev> > > > --- > > drivers/iommu/apple-dart.c | 25 +++++++++++++++++++++++++ > > drivers/pci/controller/Kconfig | 5 +++++ > > drivers/pci/controller/pcie-apple.c | 4 +++- > > 3 files changed, 33 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c > > index 559db9259e65..d1456663688e 100644 > > --- a/drivers/iommu/apple-dart.c > > +++ b/drivers/iommu/apple-dart.c > > @@ -721,6 +721,29 @@ static int apple_dart_def_domain_type(struct device *dev) > > return 0; > > } > > > > +#define DOORBELL_ADDR (CONFIG_PCIE_APPLE_MSI_DOORBELL_ADDR & PAGE_MASK) > > + > > +static void apple_dart_get_resv_regions(struct device *dev, > > + struct list_head *head) > > +{ > > +#ifdef CONFIG_PCIE_APPLE > > I think using IS_ENABLED would be better here in case the pcie > driver is built as a module which would then only define > CONFIG_PCIE_APPLE_MODULE AIUI. You're right, this isn't great. However, IS_ENABLED() still results in the evaluation of the following code by the compiler, even if it would be dropped by the optimiser. I resorted to the following construct: <quote> #ifndef CONFIG_PCIE_APPLE_MSI_DOORBELL_ADDR /* Keep things compiling when CONFIG_PCI_APPLE isn't selected */ #define CONFIG_PCIE_APPLE_MSI_DOORBELL_ADDR 0 #endif #define DOORBELL_ADDR (CONFIG_PCIE_APPLE_MSI_DOORBELL_ADDR & PAGE_MASK) static void apple_dart_get_resv_regions(struct device *dev, struct list_head *head) { if (IS_ENABLED(CONFIG_PCIE_APPLE) && dev_is_pci(dev)) { </quote> which is ugly, but works. The alternative is, of course, to add a new include file for one single value, which I find even worse. Thanks, M.
On Mon, 13 Sep 2021 19:55:53 +0100, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote: > > > +config PCIE_APPLE_MSI_DOORBELL_ADDR > > + hex > > + default 0xfffff000 > > + depends on PCIE_APPLE > > + > > config PCIE_APPLE > > tristate "Apple PCIe controller" > > depends on ARCH_APPLE || COMPILE_TEST > > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c > > index 1ed7b90f8360..76344223245d 100644 > > --- a/drivers/pci/controller/pcie-apple.c > > +++ b/drivers/pci/controller/pcie-apple.c > > @@ -120,8 +120,10 @@ > > * The doorbell address is set to 0xfffff000, which by convention > > * matches what MacOS does, and it is possible to use any other > > * address (in the bottom 4GB, as the base register is only 32bit). > > + * However, it has to be excluded from the the IOVA range, and the > > + * DART driver has to know about it. > > */ > > -#define DOORBELL_ADDR 0xfffff000 > > +#define DOORBELL_ADDR CONFIG_PCIE_APPLE_MSI_DOORBELL_ADDR > > I'm unsure if Kconfig is the right place for this. But if it is, these > hunks should be moved earlier in the series (so the deletion gets > squashed away of the hardcoded-in-the-C.) I'd rather not doing that. There is a progression in the series, and moving the value over to Kconfig is part of that progression. Thanks, M.
On 17/09/2021 18.20, Marc Zyngier wrote: > On Mon, 13 Sep 2021 21:48:47 +0100, >> On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: >>> +static inline void rmwl(u32 clr, u32 set, void __iomem *addr) >>> +{ >>> + writel_relaxed((readl_relaxed(addr) & ~clr) | set, addr); >>> +} >> >> This helper is a bit strange, especially since it's always only used >> with either clr != 0 or set != 0 but never (clr = 0 and set = 0) afaict. >> Maybe create two instead for setting and clearing bits? > > That's indeed nicer, and it makes the code more readable. This seems to be a pattern in Corellium code; the cpufreq code is the same and I honestly found it quite hard to read when used for simple set or clear operations. I find set32/clear32 style helpers much more readable, so it's probably a good idea to make this change any time we upstream stuff derived from their tree.
Thanks for giving this another push, the changes look great. The series is Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> On Mon, Sep 13, 2021 at 07:25:40PM +0100, Marc Zyngier wrote: > I have resumed my earlier effort to bring the Apple-M1 into the world > of living by equipping it with a PCIe controller driver. Huge thanks > to Alyssa Rosenzweig for kicking it into shape and providing the first > two versions of this series. > > Much has changed since v2[2]. Mark Kettenis is doing a great job with > the binding [0], so I have dropped that from the series, and strictly > focused on the Linux side of thing. I am now using this binding as is, > with the exception of a single line change, which I believe is a fix > [1]. > > Supporting the per-port interrupt controller has brought in a couple > of fixes for the core DT code. Also, some work has gone into dealing > with excluding the MSI page from the IOVA range, as well as > programming the RID-to-SID mapper. > > Overall, the driver is now much cleaner and most probably feature > complete when it comes to supporting internal devices (although I > haven't investigated things like power management). TB support is > another story, and will require some more hacking. > > This of course still depends on the clock and pinctrl drivers that are > otherwise in flight, and will affect this driver one way or another. > I have pushed a branch with all the dependencies (and more) at [3]. > > * From v2 [2]: > - Refactor DT parsing to match the new version of the binding > - Add support for INTx and port-private interrupts > - Signal link-up/down using interrupts > - Export of_phandle_args_to_fwspec > - Fix generic parsing of interrupt map > - Rationalise port setup (data structure, self discovery) > - Tell DART to exclude MSI doorbell from the IOVA mappings > - Get rid of the setup bypass if the link was found up on boot > - Prevent the module from being removed > - Program the RID-to-SID mapper on device discovery > - Rebased on 5.15-rc1 > > [0] https://lore.kernel.org/r/20210827171534.62380-1-mark.kettenis@xs4all.nl > [1] https://lore.kernel.org/r/871r5tcwhp.wl-maz@kernel.org > [2] https://lore.kernel.org/r/20210816031621.240268-1-alyssa@rosenzweig.io > [3] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=hack/m1-pcie-v3 > > Alyssa Rosenzweig (2): > PCI: apple: Add initial hardware bring-up > PCI: apple: Set up reference clocks when probing > > Marc Zyngier (8): > irqdomain: Make of_phandle_args_to_fwspec generally available > of/irq: Allow matching of an interrupt-map local to an interrupt > controller > PCI: of: Allow matching of an interrupt-map local to a pci device > PCI: apple: Add INTx and per-port interrupt support > arm64: apple: t8103: Add root port interrupt routing > PCI: apple: Implement MSI support > iommu/dart: Exclude MSI doorbell from PCIe device IOVA range > PCI: apple: Configure RID to SID mapper on device addition > > MAINTAINERS | 7 + > arch/arm64/boot/dts/apple/t8103.dtsi | 33 +- > drivers/iommu/apple-dart.c | 25 + > drivers/of/irq.c | 17 +- > drivers/pci/controller/Kconfig | 17 + > drivers/pci/controller/Makefile | 1 + > drivers/pci/controller/pcie-apple.c | 818 +++++++++++++++++++++++++++ > drivers/pci/of.c | 10 +- > include/linux/irqdomain.h | 4 + > kernel/irq/irqdomain.c | 6 +- > 10 files changed, 925 insertions(+), 13 deletions(-) > create mode 100644 drivers/pci/controller/pcie-apple.c > > -- > 2.30.2 >
I have resumed my earlier effort to bring the Apple-M1 into the world of living by equipping it with a PCIe controller driver. Huge thanks to Alyssa Rosenzweig for kicking it into shape and providing the first two versions of this series. Much has changed since v2[2]. Mark Kettenis is doing a great job with the binding [0], so I have dropped that from the series, and strictly focused on the Linux side of thing. I am now using this binding as is, with the exception of a single line change, which I believe is a fix [1]. Supporting the per-port interrupt controller has brought in a couple of fixes for the core DT code. Also, some work has gone into dealing with excluding the MSI page from the IOVA range, as well as programming the RID-to-SID mapper. Overall, the driver is now much cleaner and most probably feature complete when it comes to supporting internal devices (although I haven't investigated things like power management). TB support is another story, and will require some more hacking. This of course still depends on the clock and pinctrl drivers that are otherwise in flight, and will affect this driver one way or another. I have pushed a branch with all the dependencies (and more) at [3]. * From v2 [2]: - Refactor DT parsing to match the new version of the binding - Add support for INTx and port-private interrupts - Signal link-up/down using interrupts - Export of_phandle_args_to_fwspec - Fix generic parsing of interrupt map - Rationalise port setup (data structure, self discovery) - Tell DART to exclude MSI doorbell from the IOVA mappings - Get rid of the setup bypass if the link was found up on boot - Prevent the module from being removed - Program the RID-to-SID mapper on device discovery - Rebased on 5.15-rc1 [0] https://lore.kernel.org/r/20210827171534.62380-1-mark.kettenis@xs4all.nl [1] https://lore.kernel.org/r/871r5tcwhp.wl-maz@kernel.org [2] https://lore.kernel.org/r/20210816031621.240268-1-alyssa@rosenzweig.io [3] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=hack/m1-pcie-v3 Alyssa Rosenzweig (2): PCI: apple: Add initial hardware bring-up PCI: apple: Set up reference clocks when probing Marc Zyngier (8): irqdomain: Make of_phandle_args_to_fwspec generally available of/irq: Allow matching of an interrupt-map local to an interrupt controller PCI: of: Allow matching of an interrupt-map local to a pci device PCI: apple: Add INTx and per-port interrupt support arm64: apple: t8103: Add root port interrupt routing PCI: apple: Implement MSI support iommu/dart: Exclude MSI doorbell from PCIe device IOVA range PCI: apple: Configure RID to SID mapper on device addition MAINTAINERS | 7 + arch/arm64/boot/dts/apple/t8103.dtsi | 33 +- drivers/iommu/apple-dart.c | 25 + drivers/of/irq.c | 17 +- drivers/pci/controller/Kconfig | 17 + drivers/pci/controller/Makefile | 1 + drivers/pci/controller/pcie-apple.c | 818 +++++++++++++++++++++++++++ drivers/pci/of.c | 10 +- include/linux/irqdomain.h | 4 + kernel/irq/irqdomain.c | 6 +- 10 files changed, 925 insertions(+), 13 deletions(-) create mode 100644 drivers/pci/controller/pcie-apple.c