lkml.org 
[lkml]   [2021]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 04/10] PCI: apple: Add initial hardware bring-up


    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

    \
     
     \ /
      Last update: 2021-09-13 22:52    [W:4.601 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site