On Mon, 13 Sep 2021 21:48:47 +0100, "Sven Peter" <sven@xxxxxxxxxxxxx> wrote: > > > > On Mon, Sep 13, 2021, at 20:25, Marc Zyngier wrote: > > From: Alyssa Rosenzweig <alyssa@xxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxx> > > Signed-off-by: Stan Skowronek <stan@xxxxxxxxxxxxx> > > Signed-off-by: Alyssa Rosenzweig <alyssa@xxxxxxxxxxxxx> > > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> > > Link: https://lore.kernel.org/r/20210816031621.240268-3-alyssa@xxxxxxxxxxxxx > > --- > > 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@xxxxxxxxxxxxx> > > +M: Marc Zyngier <maz@xxxxxxxxxx> > > +L: linux-pci@xxxxxxxxxxxxxxx > > +S: Maintained > > +F: drivers/pci/controller/pcie-apple.c > > + > > APPLE SMC DRIVER > > M: Henrik Rydberg <rydberg@xxxxxxxxxxx> > > L: linux-hwmon@xxxxxxxxxxxxxxx > > 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@xxxxxxxxxxxxx> > > + * Copyright (C) 2021 Google LLC > > + * Copyright (C) 2021 Corellium LLC > > + * Copyright (C) 2021 Mark Kettenis <kettenis@xxxxxxxxxxx> > > + * > > + * Author: Alyssa Rosenzweig <alyssa@xxxxxxxxxxxxx> > > + * Author: Marc Zyngier <maz@xxxxxxxxxx> > > + */ > > + > > [...] > > + > > +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. -- Without deviation from the norm, progress is not possible.
