linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/10] PCI: Add support for Apple M1
@ 2021-09-22 20:54 Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 01/10] irqdomain: Make of_phandle_args_to_fwspec generally available Marc Zyngier
                   ` (10 more replies)
  0 siblings, 11 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team

This is v4 of the series adding PCIe support for the M1 SoC. Not a lot
has changed this time around, and most of what I was saying in [1] is
still valid.

The most important change is that the driver now probes for the number
of RID-SID mapping registers instead of assuming 64 entries. The rest
is a bunch of limited cleanups and minor fixes.

This should now be in a state that makes it mergeable, although I
expect that some of the clock bits may have to be adapted (I haven't
followed the recent developments on that front).

As always, comments welcome.

[1] https://lore.kernel.org/r/20210913182550.264165-1-maz@kernel.org

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           |  27 +
 drivers/of/irq.c                     |  17 +-
 drivers/pci/controller/Kconfig       |  17 +
 drivers/pci/controller/Makefile      |   1 +
 drivers/pci/controller/pcie-apple.c  | 826 +++++++++++++++++++++++++++
 drivers/pci/of.c                     |  10 +-
 include/linux/irqdomain.h            |   4 +
 kernel/irq/irqdomain.c               |   6 +-
 10 files changed, 935 insertions(+), 13 deletions(-)
 create mode 100644 drivers/pci/controller/pcie-apple.c

-- 
2.30.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 01/10] irqdomain: Make of_phandle_args_to_fwspec generally available
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 02/10] of/irq: Allow matching of an interrupt-map local to an interrupt controller Marc Zyngier
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team

of_phandle_args_to_fwspec() can be generally useful to code
extracting a DT of_phandle and using an irq_fwspec to use the
hierarchical irqdomain API.

Make it visible the the rest of the kernel, including modules.

Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/irqdomain.h | 4 ++++
 kernel/irq/irqdomain.c    | 6 +++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 23e4ee523576..cfd442316f39 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -64,6 +64,10 @@ struct irq_fwspec {
 	u32 param[IRQ_DOMAIN_IRQ_SPEC_PARAMS];
 };
 
+/* Conversion function from of_phandle_args fields to fwspec  */
+void of_phandle_args_to_fwspec(struct device_node *np, const u32 *args,
+			       unsigned int count, struct irq_fwspec *fwspec);
+
 /*
  * Should several domains have the same device node, but serve
  * different purposes (for example one domain is for PCI/MSI, and the
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 19e83e9b723c..5a698c1f6cc6 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -744,9 +744,8 @@ static int irq_domain_translate(struct irq_domain *d,
 	return 0;
 }
 
-static void of_phandle_args_to_fwspec(struct device_node *np, const u32 *args,
-				      unsigned int count,
-				      struct irq_fwspec *fwspec)
+void of_phandle_args_to_fwspec(struct device_node *np, const u32 *args,
+			       unsigned int count, struct irq_fwspec *fwspec)
 {
 	int i;
 
@@ -756,6 +755,7 @@ static void of_phandle_args_to_fwspec(struct device_node *np, const u32 *args,
 	for (i = 0; i < count; i++)
 		fwspec->param[i] = args[i];
 }
+EXPORT_SYMBOL_GPL(of_phandle_args_to_fwspec);
 
 unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 02/10] of/irq: Allow matching of an interrupt-map local to an interrupt controller
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 01/10] irqdomain: Make of_phandle_args_to_fwspec generally available Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 03/10] PCI: of: Allow matching of an interrupt-map local to a PCI device Marc Zyngier
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team, Rob Herring

of_irq_parse_raw() has a baked assumption that if a node has an
interrupt-controller property, it cannot possibly also have an
interrupt-map property (the latter being ignored).

This seems to be an odd behaviour, and there is no reason why
we should avoid supporting this use case. This is specially
useful when a PCI root port acts as an interrupt controller for
PCI endpoints, such as this:

pcie0: pcie@690000000 {
	[...]
	port00: pci@0,0 {
		device_type = "pci";
		[...]
		#address-cells = <3>;

		interrupt-controller;
		#interrupt-cells = <1>;

		interrupt-map-mask = <0 0 0 7>;
		interrupt-map = <0 0 0 1 &port00 0 0 0 0>,
				<0 0 0 2 &port00 0 0 0 1>,
				<0 0 0 3 &port00 0 0 0 2>,
				<0 0 0 4 &port00 0 0 0 3>;
	};
};

Handle it by detecting that we have an interrupt-map early in the
parsing, and special case the situation where the phandle in the
interrupt map refers to the current node (which is the interesting
case here).

Reviewed-by: Rob Herring <robh@kernel.org>
Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/of/irq.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 352e14b007e7..32be5a03951f 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -156,10 +156,14 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 
 	/* Now start the actual "proper" walk of the interrupt tree */
 	while (ipar != NULL) {
-		/* Now check if cursor is an interrupt-controller and if it is
-		 * then we are done
+		/*
+		 * Now check if cursor is an interrupt-controller and
+		 * if it is then we are done, unless there is an
+		 * interrupt-map which takes precedence.
 		 */
-		if (of_property_read_bool(ipar, "interrupt-controller")) {
+		imap = of_get_property(ipar, "interrupt-map", &imaplen);
+		if (imap == NULL &&
+		    of_property_read_bool(ipar, "interrupt-controller")) {
 			pr_debug(" -> got it !\n");
 			return 0;
 		}
@@ -173,8 +177,6 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 			goto fail;
 		}
 
-		/* Now look for an interrupt-map */
-		imap = of_get_property(ipar, "interrupt-map", &imaplen);
 		/* No interrupt map, check for an interrupt parent */
 		if (imap == NULL) {
 			pr_debug(" -> no map, getting parent\n");
@@ -255,6 +257,11 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 		out_irq->args_count = intsize = newintsize;
 		addrsize = newaddrsize;
 
+		if (ipar == newpar) {
+			pr_debug("%pOF interrupt-map entry to self\n", ipar);
+			return 0;
+		}
+
 	skiplevel:
 		/* Iterate again with new parent */
 		out_irq->np = newpar;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 03/10] PCI: of: Allow matching of an interrupt-map local to a PCI device
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 01/10] irqdomain: Make of_phandle_args_to_fwspec generally available Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 02/10] of/irq: Allow matching of an interrupt-map local to an interrupt controller Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up Marc Zyngier
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team, Rob Herring

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.

Allow the matching of such property when local to the node of a PCI
device, which allows the device itself to use the interrupt map for for
its own purpose.

Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
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..0b1237cff239 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 (of_get_property(dn, "interrupt-map", NULL)) {
+		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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
                   ` (2 preceding siblings ...)
  2021-09-22 20:54 ` [PATCH v4 03/10] PCI: of: Allow matching of an interrupt-map local to a PCI device Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 21:08   ` Sven Peter
  2021-09-22 21:09   ` Rob Herring
  2021-09-22 20:54 ` [PATCH v4 05/10] PCI: apple: Set up reference clocks when probing Marc Zyngier
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team

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>
---
 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 ba3e2419e86a..18fc0ed9a7f6 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..2479a4168439
--- /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>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/iopoll.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of_irq.h>
+#include <linux/pci-ecam.h>
+
+#define CORE_RC_PHYIF_CTL		0x00024
+#define   CORE_RC_PHYIF_CTL_RUN		BIT(0)
+#define CORE_RC_PHYIF_STAT		0x00028
+#define   CORE_RC_PHYIF_STAT_REFCLK	BIT(4)
+#define CORE_RC_CTL			0x00050
+#define   CORE_RC_CTL_RUN		BIT(0)
+#define CORE_RC_STAT			0x00058
+#define   CORE_RC_STAT_READY		BIT(0)
+#define CORE_FABRIC_STAT		0x04000
+#define   CORE_FABRIC_STAT_MASK		0x001F001F
+#define CORE_LANE_CFG(port)		(0x84000 + 0x4000 * (port))
+#define   CORE_LANE_CFG_REFCLK0REQ	BIT(0)
+#define   CORE_LANE_CFG_REFCLK1		BIT(1)
+#define   CORE_LANE_CFG_REFCLK0ACK	BIT(2)
+#define   CORE_LANE_CFG_REFCLKEN	(BIT(9) | BIT(10))
+#define CORE_LANE_CTL(port)		(0x84004 + 0x4000 * (port))
+#define   CORE_LANE_CTL_CFGACC		BIT(15)
+
+#define PORT_LTSSMCTL			0x00080
+#define   PORT_LTSSMCTL_START		BIT(0)
+#define PORT_INTSTAT			0x00100
+#define   PORT_INT_TUNNEL_ERR		31
+#define   PORT_INT_CPL_TIMEOUT		23
+#define   PORT_INT_RID2SID_MAPERR	22
+#define   PORT_INT_CPL_ABORT		21
+#define   PORT_INT_MSI_BAD_DATA		19
+#define   PORT_INT_MSI_ERR		18
+#define   PORT_INT_REQADDR_GT32		17
+#define   PORT_INT_AF_TIMEOUT		15
+#define   PORT_INT_LINK_DOWN		14
+#define   PORT_INT_LINK_UP		12
+#define   PORT_INT_LINK_BWMGMT		11
+#define   PORT_INT_AER_MASK		(15 << 4)
+#define   PORT_INT_PORT_ERR		4
+#define   PORT_INT_INTx(i)		i
+#define   PORT_INT_INTx_MASK		15
+#define PORT_INTMSK			0x00104
+#define PORT_INTMSKSET			0x00108
+#define PORT_INTMSKCLR			0x0010c
+#define PORT_MSICFG			0x00124
+#define   PORT_MSICFG_EN		BIT(0)
+#define   PORT_MSICFG_L2MSINUM_SHIFT	4
+#define PORT_MSIBASE			0x00128
+#define   PORT_MSIBASE_1_SHIFT		16
+#define PORT_MSIADDR			0x00168
+#define PORT_LINKSTS			0x00208
+#define   PORT_LINKSTS_UP		BIT(0)
+#define   PORT_LINKSTS_BUSY		BIT(2)
+#define PORT_LINKCMDSTS			0x00210
+#define PORT_OUTS_NPREQS		0x00284
+#define   PORT_OUTS_NPREQS_REQ		BIT(24)
+#define   PORT_OUTS_NPREQS_CPL		BIT(16)
+#define PORT_RXWR_FIFO			0x00288
+#define   PORT_RXWR_FIFO_HDR		GENMASK(15, 10)
+#define   PORT_RXWR_FIFO_DATA		GENMASK(9, 0)
+#define PORT_RXRD_FIFO			0x0028C
+#define   PORT_RXRD_FIFO_REQ		GENMASK(6, 0)
+#define PORT_OUTS_CPLS			0x00290
+#define   PORT_OUTS_CPLS_SHRD		GENMASK(14, 8)
+#define   PORT_OUTS_CPLS_WAIT		GENMASK(6, 0)
+#define PORT_APPCLK			0x00800
+#define   PORT_APPCLK_EN		BIT(0)
+#define   PORT_APPCLK_CGDIS		BIT(8)
+#define PORT_STATUS			0x00804
+#define   PORT_STATUS_READY		BIT(0)
+#define PORT_REFCLK			0x00810
+#define   PORT_REFCLK_EN		BIT(0)
+#define   PORT_REFCLK_CGDIS		BIT(8)
+#define PORT_PERST			0x00814
+#define   PORT_PERST_OFF		BIT(0)
+#define PORT_RID2SID(i16)		(0x00828 + 4 * (i16))
+#define   PORT_RID2SID_VALID		BIT(31)
+#define   PORT_RID2SID_SID_SHIFT	16
+#define   PORT_RID2SID_BUS_SHIFT	8
+#define   PORT_RID2SID_DEV_SHIFT	3
+#define   PORT_RID2SID_FUNC_SHIFT	0
+#define PORT_OUTS_PREQS_HDR		0x00980
+#define   PORT_OUTS_PREQS_HDR_MASK	GENMASK(9, 0)
+#define PORT_OUTS_PREQS_DATA		0x00984
+#define   PORT_OUTS_PREQS_DATA_MASK	GENMASK(15, 0)
+#define PORT_TUNCTRL			0x00988
+#define   PORT_TUNCTRL_PERST_ON		BIT(0)
+#define   PORT_TUNCTRL_PERST_ACK_REQ	BIT(1)
+#define PORT_TUNSTAT			0x0098c
+#define   PORT_TUNSTAT_PERST_ON		BIT(0)
+#define   PORT_TUNSTAT_PERST_ACK_PEND	BIT(1)
+#define PORT_PREFMEM_ENABLE		0x00994
+
+struct apple_pcie {
+	struct device		*dev;
+	void __iomem            *base;
+};
+
+struct apple_pcie_port {
+	struct apple_pcie	*pcie;
+	struct device_node	*np;
+	void __iomem		*base;
+	int			idx;
+};
+
+static void rmw_set(u32 set, void __iomem *addr)
+{
+	writel_relaxed(readl_relaxed(addr) | set, addr);
+}
+
+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;
+
+	rmw_set(PORT_APPCLK_EN, port + PORT_APPCLK);
+
+	rmw_set(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();
+
+	writel_relaxed(PORT_LTSSMCTL_START, port->base + PORT_LTSSMCTL);
+
+	return 0;
+}
+
+static int apple_pcie_init(struct pci_config_window *cfg)
+{
+	struct device *dev = cfg->parent;
+	struct platform_device *platform = to_platform_device(dev);
+	struct device_node *of_port;
+	struct apple_pcie *pcie;
+	int ret;
+
+	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->dev = dev;
+
+	mutex_init(&pcie->lock);
+
+	pcie->base = devm_platform_ioremap_resource(platform, 1);
+
+	if (IS_ERR(pcie->base))
+		return -ENODEV;
+
+	for_each_child_of_node(dev->of_node, of_port) {
+		ret = apple_pcie_setup_port(pcie, of_port);
+		if (ret) {
+			dev_err(pcie->dev, "Port %pOF setup fail: %d\n", of_port, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct pci_ecam_ops apple_pcie_cfg_ecam_ops = {
+	.init		= apple_pcie_init,
+	.pci_ops	= {
+		.map_bus	= pci_ecam_map_bus,
+		.read		= pci_generic_config_read,
+		.write		= pci_generic_config_write,
+	}
+};
+
+static const struct of_device_id apple_pcie_of_match[] = {
+	{ .compatible = "apple,pcie", .data = &apple_pcie_cfg_ecam_ops },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, apple_pcie_of_match);
+
+static struct platform_driver apple_pcie_driver = {
+	.probe	= pci_host_common_probe,
+	.driver	= {
+		.name			= "pcie-apple",
+		.of_match_table		= apple_pcie_of_match,
+		.suppress_bind_attrs	= true,
+	},
+};
+module_platform_driver(apple_pcie_driver);
+
+MODULE_LICENSE("GPL v2");
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 05/10] PCI: apple: Set up reference clocks when probing
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
                   ` (3 preceding siblings ...)
  2021-09-22 20:54 ` [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 06/10] PCI: apple: Add INTx and per-port interrupt support Marc Zyngier
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team

From: Alyssa Rosenzweig <alyssa@rosenzweig.io>

Apple's PCIe controller requires clocks to be configured in order to
bring up the hardware. Add the register pokes required to do so.

Adapted from Corellium's driver via Mark Kettenis's U-Boot patches.

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>
---
 drivers/pci/controller/pcie-apple.c | 46 +++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index 2479a4168439..2620fd9d0ab1 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -132,6 +132,48 @@ static void rmw_set(u32 set, void __iomem *addr)
 	writel_relaxed(readl_relaxed(addr) | set, addr);
 }
 
+static void rmw_clear(u32 clr, void __iomem *addr)
+{
+	writel_relaxed(readl_relaxed(addr) & ~clr, addr);
+}
+
+static int apple_pcie_setup_refclk(struct apple_pcie *pcie,
+				   struct apple_pcie_port *port)
+{
+	u32 stat;
+	int res;
+
+	res = readl_relaxed_poll_timeout(pcie->base + CORE_RC_PHYIF_STAT, stat,
+					 stat & CORE_RC_PHYIF_STAT_REFCLK,
+					 100, 50000);
+	if (res < 0)
+		return res;
+
+	rmw_set(CORE_LANE_CTL_CFGACC, pcie->base + CORE_LANE_CTL(port->idx));
+	rmw_set(CORE_LANE_CFG_REFCLK0REQ, pcie->base + CORE_LANE_CFG(port->idx));
+
+	res = readl_relaxed_poll_timeout(pcie->base + CORE_LANE_CFG(port->idx),
+					 stat, stat & CORE_LANE_CFG_REFCLK0ACK,
+					 100, 50000);
+	if (res < 0)
+		return res;
+
+	rmw_set(CORE_LANE_CFG_REFCLK1, pcie->base + CORE_LANE_CFG(port->idx));
+	res = readl_relaxed_poll_timeout(pcie->base + CORE_LANE_CFG(port->idx),
+					 stat, stat & CORE_LANE_CFG_REFCLK1,
+					 100, 50000);
+
+	if (res < 0)
+		return res;
+
+	rmw_clear(CORE_LANE_CTL_CFGACC, pcie->base + CORE_LANE_CTL(port->idx));
+
+	rmw_set(CORE_LANE_CFG_REFCLKEN, pcie->base + CORE_LANE_CFG(port->idx));
+	rmw_set(PORT_REFCLK_EN, port->base + PORT_REFCLK);
+
+	return 0;
+}
+
 static int apple_pcie_setup_port(struct apple_pcie *pcie,
 				 struct device_node *np)
 {
@@ -165,6 +207,10 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie,
 
 	rmw_set(PORT_APPCLK_EN, port + PORT_APPCLK);
 
+	ret = apple_pcie_setup_refclk(pcie, port);
+	if (ret < 0)
+		return ret;
+
 	rmw_set(PORT_PERST_OFF, port->base + PORT_PERST);
 	gpiod_set_value(reset, 1);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 06/10] PCI: apple: Add INTx and per-port interrupt support
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
                   ` (4 preceding siblings ...)
  2021-09-22 20:54 ` [PATCH v4 05/10] PCI: apple: Set up reference clocks when probing Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 07/10] arm64: apple: t8103: Add root port interrupt routing Marc Zyngier
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team

Add support for the per-port interrupt controller that deals
with both INTx signalling and management interrupts.

This allows the Link-up/Link-down interrupts to be wired, allowing
the bring-up to be synchronised (and provide debug information).
The framework can further be used to handle the rest of the per
port events if and when necessary.

Likewise, INTx signalling is implemented so that end-points can
actually be used.

Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/pcie-apple.c | 209 ++++++++++++++++++++++++++++
 1 file changed, 209 insertions(+)

diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index 2620fd9d0ab1..38b5e0b7b691 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -21,6 +21,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/iopoll.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/msi.h>
@@ -118,12 +119,14 @@
 struct apple_pcie {
 	struct device		*dev;
 	void __iomem            *base;
+	struct completion	event;
 };
 
 struct apple_pcie_port {
 	struct apple_pcie	*pcie;
 	struct device_node	*np;
 	void __iomem		*base;
+	struct irq_domain	*domain;
 	int			idx;
 };
 
@@ -137,6 +140,200 @@ static void rmw_clear(u32 clr, void __iomem *addr)
 	writel_relaxed(readl_relaxed(addr) & ~clr, addr);
 }
 
+static void apple_port_irq_mask(struct irq_data *data)
+{
+	struct apple_pcie_port *port = irq_data_get_irq_chip_data(data);
+
+	writel_relaxed(BIT(data->hwirq), port->base + PORT_INTMSKSET);
+}
+
+static void apple_port_irq_unmask(struct irq_data *data)
+{
+	struct apple_pcie_port *port = irq_data_get_irq_chip_data(data);
+
+	writel_relaxed(BIT(data->hwirq), port->base + PORT_INTMSKCLR);
+}
+
+static bool hwirq_is_intx(unsigned int hwirq)
+{
+	return BIT(hwirq) & PORT_INT_INTx_MASK;
+}
+
+static void apple_port_irq_ack(struct irq_data *data)
+{
+	struct apple_pcie_port *port = irq_data_get_irq_chip_data(data);
+
+	if (!hwirq_is_intx(data->hwirq))
+		writel_relaxed(BIT(data->hwirq), port->base + PORT_INTSTAT);
+}
+
+static int apple_port_irq_set_type(struct irq_data *data, unsigned int type)
+{
+	/*
+	 * It doesn't seem that there is any way to configure the
+	 * trigger, so assume INTx have to be level (as per the spec),
+	 * and the rest is edge (which looks likely).
+	 */
+	if (hwirq_is_intx(data->hwirq) ^ !!(type & IRQ_TYPE_LEVEL_MASK))
+		return -EINVAL;
+
+	irqd_set_trigger_type(data, type);
+	return 0;
+}
+
+static struct irq_chip apple_port_irqchip = {
+	.name		= "PCIe",
+	.irq_ack	= apple_port_irq_ack,
+	.irq_mask	= apple_port_irq_mask,
+	.irq_unmask	= apple_port_irq_unmask,
+	.irq_set_type	= apple_port_irq_set_type,
+};
+
+static int apple_port_irq_domain_alloc(struct irq_domain *domain,
+				       unsigned int virq, unsigned int nr_irqs,
+				       void *args)
+{
+	struct apple_pcie_port *port = domain->host_data;
+	struct irq_fwspec *fwspec = args;
+	int i;
+
+	for (i = 0; i < nr_irqs; i++) {
+		irq_flow_handler_t flow = handle_edge_irq;
+		unsigned int type = IRQ_TYPE_EDGE_RISING;
+
+		if (hwirq_is_intx(fwspec->param[0] + i)) {
+			flow = handle_level_irq;
+			type = IRQ_TYPE_LEVEL_HIGH;
+		}
+
+		irq_domain_set_info(domain, virq + i, fwspec->param[0] + i,
+				    &apple_port_irqchip, port, flow,
+				    NULL, NULL);
+
+		irq_set_irq_type(virq + i, type);
+	}
+
+	return 0;
+}
+
+static void apple_port_irq_domain_free(struct irq_domain *domain,
+				       unsigned int virq, unsigned int nr_irqs)
+{
+	int i;
+
+	for (i = 0; i < nr_irqs; i++) {
+		struct irq_data *d = irq_domain_get_irq_data(domain, virq + i);
+		irq_set_handler(virq + i, NULL);
+		irq_domain_reset_irq_data(d);
+	}
+}
+
+static const struct irq_domain_ops apple_port_irq_domain_ops = {
+	.translate	= irq_domain_translate_onecell,
+	.alloc		= apple_port_irq_domain_alloc,
+	.free		= apple_port_irq_domain_free,
+};
+
+static void apple_port_irq_handler(struct irq_desc *desc)
+{
+	struct apple_pcie_port *port = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned long stat;
+	int i;
+
+	chained_irq_enter(chip, desc);
+
+	stat = readl_relaxed(port->base + PORT_INTSTAT);
+
+	for_each_set_bit(i, &stat, 32)
+		generic_handle_domain_irq(port->domain, i);
+
+	chained_irq_exit(chip, desc);
+}
+
+static int apple_pcie_port_setup_irq(struct apple_pcie_port *port)
+{
+	struct fwnode_handle *fwnode = &port->np->fwnode;
+	unsigned int irq;
+
+	/* FIXME: consider moving each interrupt under each port */
+	irq = irq_of_parse_and_map(to_of_node(dev_fwnode(port->pcie->dev)),
+				   port->idx);
+	if (!irq)
+		return -ENXIO;
+
+	port->domain = irq_domain_create_linear(fwnode, 32,
+						&apple_port_irq_domain_ops,
+						port);
+	if (!port->domain)
+		return -ENOMEM;
+
+	/* Disable all interrupts */
+	writel_relaxed(~0, port->base + PORT_INTMSKSET);
+	writel_relaxed(~0, port->base + PORT_INTSTAT);
+
+	irq_set_chained_handler_and_data(irq, apple_port_irq_handler, port);
+
+	return 0;
+}
+
+static irqreturn_t apple_pcie_port_irq(int irq, void *data)
+{
+	struct apple_pcie_port *port = data;
+	unsigned int hwirq = irq_domain_get_irq_data(port->domain, irq)->hwirq;
+
+	switch (hwirq) {
+	case PORT_INT_LINK_UP:
+		dev_info_ratelimited(port->pcie->dev, "Link up on %pOF\n",
+				     port->np);
+		complete_all(&port->pcie->event);
+		break;
+	case PORT_INT_LINK_DOWN:
+		dev_info_ratelimited(port->pcie->dev, "Link down on %pOF\n",
+				     port->np);
+		break;
+	default:
+		return IRQ_NONE;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int apple_pcie_port_register_irqs(struct apple_pcie_port *port)
+{
+	static struct {
+		unsigned int	hwirq;
+		const char	*name;
+	} port_irqs[] = {
+		{ PORT_INT_LINK_UP,	"Link up",	},
+		{ PORT_INT_LINK_DOWN,	"Link down",	},
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(port_irqs); i++) {
+		struct irq_fwspec fwspec = {
+			.fwnode		= &port->np->fwnode,
+			.param_count	= 1,
+			.param		= {
+				[0]	= port_irqs[i].hwirq,
+			},
+		};
+		unsigned int irq;
+		int ret;
+
+		irq = irq_domain_alloc_irqs(port->domain, 1, NUMA_NO_NODE,
+					    &fwspec);
+		if (WARN_ON(!irq))
+			continue;
+
+		ret = request_irq(irq, apple_pcie_port_irq, 0,
+				  port_irqs[i].name, port);
+		WARN_ON(ret);
+	}
+
+	return 0;
+}
+
 static int apple_pcie_setup_refclk(struct apple_pcie *pcie,
 				   struct apple_pcie_port *port)
 {
@@ -224,8 +421,20 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie,
 	/* Flush writes and enable the link */
 	dma_wmb();
 
+	ret = apple_pcie_port_setup_irq(port);
+	if (ret)
+		return ret;
+
+	init_completion(&pcie->event);
+
+	ret = apple_pcie_port_register_irqs(port);
+	WARN_ON(ret);
+
 	writel_relaxed(PORT_LTSSMCTL_START, port->base + PORT_LTSSMCTL);
 
+	if (!wait_for_completion_timeout(&pcie->event, HZ / 10))
+		dev_warn(pcie->dev, "%pOF link didn't come up\n", np);
+
 	return 0;
 }
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 07/10] arm64: apple: t8103: Add root port interrupt routing
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
                   ` (5 preceding siblings ...)
  2021-09-22 20:54 ` [PATCH v4 06/10] PCI: apple: Add INTx and per-port interrupt support Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 08/10] PCI: apple: Implement MSI support Marc Zyngier
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team

Add the interrupt-map properties that are required for INTx
signalling.

Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/boot/dts/apple/t8103.dtsi | 33 +++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi
index 085b4e9b6a6a..a63a3d4955f8 100644
--- a/arch/arm64/boot/dts/apple/t8103.dtsi
+++ b/arch/arm64/boot/dts/apple/t8103.dtsi
@@ -382,7 +382,7 @@ pcie0: pcie@690000000 {
 			pinctrl-0 = <&pcie_pins>;
 			pinctrl-names = "default";
 
-			pci@0,0 {
+			port00: pci@0,0 {
 				device_type = "pci";
 				reg = <0x0 0x0 0x0 0x0 0x0>;
 				reset-gpios = <&pinctrl_ap 152 0>;
@@ -391,9 +391,18 @@ pci@0,0 {
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
+
+				interrupt-controller;
+				#interrupt-cells = <1>;
+
+				interrupt-map-mask = <0 0 0 7>;
+				interrupt-map = <0 0 0 1 &port00 0 0 0 0>,
+						<0 0 0 2 &port00 0 0 0 1>,
+						<0 0 0 3 &port00 0 0 0 2>,
+						<0 0 0 4 &port00 0 0 0 3>;
 			};
 
-			pci@1,0 {
+			port01: pci@1,0 {
 				device_type = "pci";
 				reg = <0x800 0x0 0x0 0x0 0x0>;
 				reset-gpios = <&pinctrl_ap 153 0>;
@@ -402,9 +411,18 @@ pci@1,0 {
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
+
+				interrupt-controller;
+				#interrupt-cells = <1>;
+
+				interrupt-map-mask = <0 0 0 7>;
+				interrupt-map = <0 0 0 1 &port01 0 0 0 0>,
+						<0 0 0 2 &port01 0 0 0 1>,
+						<0 0 0 3 &port01 0 0 0 2>,
+						<0 0 0 4 &port01 0 0 0 3>;
 			};
 
-			pci@2,0 {
+			port02: pci@2,0 {
 				device_type = "pci";
 				reg = <0x1000 0x0 0x0 0x0 0x0>;
 				reset-gpios = <&pinctrl_ap 33 0>;
@@ -413,6 +431,15 @@ pci@2,0 {
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
+
+				interrupt-controller;
+				#interrupt-cells = <1>;
+
+				interrupt-map-mask = <0 0 0 7>;
+				interrupt-map = <0 0 0 1 &port02 0 0 0 0>,
+						<0 0 0 2 &port02 0 0 0 1>,
+						<0 0 0 3 &port02 0 0 0 2>,
+						<0 0 0 4 &port02 0 0 0 3>;
 			};
 		};
 	};
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 08/10] PCI: apple: Implement MSI support
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
                   ` (6 preceding siblings ...)
  2021-09-22 20:54 ` [PATCH v4 07/10] arm64: apple: t8103: Add root port interrupt routing Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 09/10] iommu/dart: Exclude MSI doorbell from PCIe device IOVA range Marc Zyngier
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team

Probe for the 'msi-ranges' property, and implement the MSI
support in the form of the usual two-level hierarchy.

Note that contrary to the wired interrupts, MSIs are shared among
all the ports.

Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/pcie-apple.c | 167 +++++++++++++++++++++++++++-
 1 file changed, 166 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index 38b5e0b7b691..de1cbc28d849 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -116,10 +116,22 @@
 #define   PORT_TUNSTAT_PERST_ACK_PEND	BIT(1)
 #define PORT_PREFMEM_ENABLE		0x00994
 
+/*
+ * 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).
+ */
+#define DOORBELL_ADDR			0xfffff000
+
 struct apple_pcie {
+	struct mutex		lock;
 	struct device		*dev;
 	void __iomem            *base;
+	struct irq_domain	*domain;
+	unsigned long		*bitmap;
 	struct completion	event;
+	struct irq_fwspec	fwspec;
+	u32			nvecs;
 };
 
 struct apple_pcie_port {
@@ -140,6 +152,101 @@ static void rmw_clear(u32 clr, void __iomem *addr)
 	writel_relaxed(readl_relaxed(addr) & ~clr, addr);
 }
 
+static void apple_msi_top_irq_mask(struct irq_data *d)
+{
+	pci_msi_mask_irq(d);
+	irq_chip_mask_parent(d);
+}
+
+static void apple_msi_top_irq_unmask(struct irq_data *d)
+{
+	pci_msi_unmask_irq(d);
+	irq_chip_unmask_parent(d);
+}
+
+static struct irq_chip apple_msi_top_chip = {
+	.name			= "PCIe MSI",
+	.irq_mask		= apple_msi_top_irq_mask,
+	.irq_unmask		= apple_msi_top_irq_unmask,
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+	.irq_set_type		= irq_chip_set_type_parent,
+};
+
+static void apple_msi_compose_msg(struct irq_data *data, struct msi_msg *msg)
+{
+	msg->address_hi = upper_32_bits(DOORBELL_ADDR);
+	msg->address_lo = lower_32_bits(DOORBELL_ADDR);
+	msg->data = data->hwirq;
+}
+
+static struct irq_chip apple_msi_bottom_chip = {
+	.name			= "MSI",
+	.irq_mask		= irq_chip_mask_parent,
+	.irq_unmask		= irq_chip_unmask_parent,
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+	.irq_set_type		= irq_chip_set_type_parent,
+	.irq_compose_msi_msg	= apple_msi_compose_msg,
+};
+
+static int apple_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				  unsigned int nr_irqs, void *args)
+{
+	struct apple_pcie *pcie = domain->host_data;
+	struct irq_fwspec fwspec = pcie->fwspec;
+	unsigned int i;
+	int ret, hwirq;
+
+	mutex_lock(&pcie->lock);
+
+	hwirq = bitmap_find_free_region(pcie->bitmap, pcie->nvecs,
+					order_base_2(nr_irqs));
+
+	mutex_unlock(&pcie->lock);
+
+	if (hwirq < 0)
+		return -ENOSPC;
+
+	fwspec.param[1] += hwirq;
+
+	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &fwspec);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < nr_irqs; i++) {
+		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+					      &apple_msi_bottom_chip,
+					      domain->host_data);
+	}
+
+	return 0;
+}
+
+static void apple_msi_domain_free(struct irq_domain *domain, unsigned int virq,
+				  unsigned int nr_irqs)
+{
+	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
+	struct apple_pcie *pcie = domain->host_data;
+
+	mutex_lock(&pcie->lock);
+
+	bitmap_release_region(pcie->bitmap, d->hwirq, order_base_2(nr_irqs));
+
+	mutex_unlock(&pcie->lock);
+}
+
+static const struct irq_domain_ops apple_msi_domain_ops = {
+	.alloc	= apple_msi_domain_alloc,
+	.free	= apple_msi_domain_free,
+};
+
+static struct msi_domain_info apple_msi_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		   MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
+	.chip	= &apple_msi_top_chip,
+};
+
 static void apple_port_irq_mask(struct irq_data *data)
 {
 	struct apple_pcie_port *port = irq_data_get_irq_chip_data(data);
@@ -274,6 +381,15 @@ 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 */
+	BUILD_BUG_ON(upper_32_bits(DOORBELL_ADDR));
+	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;
 }
 
@@ -438,6 +554,55 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie,
 	return 0;
 }
 
+static int apple_msi_init(struct apple_pcie *pcie)
+{
+	struct fwnode_handle *fwnode = dev_fwnode(pcie->dev);
+	struct of_phandle_args args = {};
+	struct irq_domain *parent;
+	int ret;
+
+	ret = of_parse_phandle_with_args(to_of_node(fwnode), "msi-ranges",
+					 "#interrupt-cells", 0, &args);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32_index(to_of_node(fwnode), "msi-ranges",
+					 args.args_count + 1, &pcie->nvecs);
+	if (ret)
+		return ret;
+
+	of_phandle_args_to_fwspec(args.np, args.args, args.args_count,
+				  &pcie->fwspec);
+
+	pcie->bitmap = devm_bitmap_zalloc(pcie->dev, pcie->nvecs, GFP_KERNEL);
+	if (!pcie->bitmap)
+		return -ENOMEM;
+
+	parent = irq_find_matching_fwspec(&pcie->fwspec, DOMAIN_BUS_WIRED);
+	if (!parent) {
+		dev_err(pcie->dev, "failed to find parent domain\n");
+		return -ENXIO;
+	}
+
+	parent = irq_domain_create_hierarchy(parent, 0, pcie->nvecs, fwnode,
+					     &apple_msi_domain_ops, pcie);
+	if (!parent) {
+		dev_err(pcie->dev, "failed to create IRQ domain\n");
+		return -ENOMEM;
+	}
+	irq_domain_update_bus_token(parent, DOMAIN_BUS_NEXUS);
+
+	pcie->domain = pci_msi_create_irq_domain(fwnode, &apple_msi_info,
+						 parent);
+	if (!pcie->domain) {
+		dev_err(pcie->dev, "failed to create MSI domain\n");
+		irq_domain_remove(parent);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
 static int apple_pcie_init(struct pci_config_window *cfg)
 {
 	struct device *dev = cfg->parent;
@@ -467,7 +632,7 @@ static int apple_pcie_init(struct pci_config_window *cfg)
 		}
 	}
 
-	return 0;
+	return apple_msi_init(pcie);
 }
 
 static const struct pci_ecam_ops apple_pcie_cfg_ecam_ops = {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 09/10] iommu/dart: Exclude MSI doorbell from PCIe device IOVA range
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
                   ` (7 preceding siblings ...)
  2021-09-22 20:54 ` [PATCH v4 08/10] PCI: apple: Implement MSI support Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 20:54 ` [PATCH v4 10/10] PCI: apple: Configure RID to SID mapper on device addition Marc Zyngier
  2021-09-22 23:32 ` [PATCH v4 00/10] PCI: Add support for Apple M1 Mark Kettenis
  10 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team

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.

Reviewed-by: Sven Peter <sven@svenpeter.dev>
Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/iommu/apple-dart.c          | 27 +++++++++++++++++++++++++++
 drivers/pci/controller/Kconfig      |  5 +++++
 drivers/pci/controller/pcie-apple.c |  4 +++-
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index a5e98b9ef54a..6e0ae8eb0af2 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -704,6 +704,31 @@ static int apple_dart_def_domain_type(struct device *dev)
 	return 0;
 }
 
+#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)) {
+		struct iommu_resv_region *region;
+		int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+
+		region = iommu_alloc_resv_region(DOORBELL_ADDR,
+						 PAGE_SIZE, prot,
+						 IOMMU_RESV_MSI);
+		if (!region)
+			return;
+
+		list_add_tail(&region->list, head);
+	}
+
+	iommu_dma_get_resv_regions(dev, head);
+}
+
 static const struct iommu_ops apple_dart_iommu_ops = {
 	.domain_alloc = apple_dart_domain_alloc,
 	.domain_free = apple_dart_domain_free,
@@ -720,6 +745,8 @@ static const struct iommu_ops apple_dart_iommu_ops = {
 	.device_group = apple_dart_device_group,
 	.of_xlate = apple_dart_of_xlate,
 	.def_domain_type = apple_dart_def_domain_type,
+	.get_resv_regions = apple_dart_get_resv_regions,
+	.put_resv_regions = generic_iommu_put_resv_regions,
 	.pgsize_bitmap = -1UL, /* Restricted during dart probe */
 };
 
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 814833a8120d..b6e7410da254 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -312,6 +312,11 @@ 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_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 de1cbc28d849..abe94168a36d 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
 
 struct apple_pcie {
 	struct mutex		lock;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 10/10] PCI: apple: Configure RID to SID mapper on device addition
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
                   ` (8 preceding siblings ...)
  2021-09-22 20:54 ` [PATCH v4 09/10] iommu/dart: Exclude MSI doorbell from PCIe device IOVA range Marc Zyngier
@ 2021-09-22 20:54 ` Marc Zyngier
  2021-09-22 23:32 ` [PATCH v4 00/10] PCI: Add support for Apple M1 Mark Kettenis
  10 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 20:54 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	kernel-team

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.

Reviewed-by: Sven Peter <sven@svenpeter.dev>
Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/pcie-apple.c | 165 +++++++++++++++++++++++++++-
 1 file changed, 163 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index abe94168a36d..3ce3594da548 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
+
 /*
  * The doorbell address is set to 0xfffff000, which by convention
  * matches what MacOS does, and it is possible to use any other
@@ -131,6 +135,7 @@ struct apple_pcie {
 	void __iomem            *base;
 	struct irq_domain	*domain;
 	unsigned long		*bitmap;
+	struct list_head	ports;
 	struct completion	event;
 	struct irq_fwspec	fwspec;
 	u32			nvecs;
@@ -141,6 +146,9 @@ struct apple_pcie_port {
 	struct device_node	*np;
 	void __iomem		*base;
 	struct irq_domain	*domain;
+	struct list_head	entry;
+	DECLARE_BITMAP(		sid_map, MAX_RID2SID);
+	int			sid_map_sz;
 	int			idx;
 };
 
@@ -489,6 +497,14 @@ static int apple_pcie_setup_refclk(struct apple_pcie *pcie,
 	return 0;
 }
 
+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 */
+	return readl_relaxed(port->base + PORT_RID2SID(idx));
+}
+
 static int apple_pcie_setup_port(struct apple_pcie *pcie,
 				 struct device_node *np)
 {
@@ -496,7 +512,7 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie,
 	struct apple_pcie_port *port;
 	struct gpio_desc *reset;
 	u32 stat, idx;
-	int ret;
+	int ret, i;
 
 	reset = gpiod_get_from_of_node(np, "reset-gpios", 0,
 				       GPIOD_OUT_LOW, "#PERST");
@@ -543,6 +559,18 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie,
 	if (ret)
 		return ret;
 
+	/* 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);
 
 	ret = apple_pcie_port_register_irqs(port);
@@ -605,6 +633,121 @@ static int apple_msi_init(struct apple_pcie *pcie)
 	return 0;
 }
 
+static struct apple_pcie_port *apple_pcie_get_port(struct pci_dev *pdev)
+{
+	struct pci_config_window *cfg = pdev->sysdata;
+	struct apple_pcie *pcie = cfg->priv;
+	struct pci_dev *port_pdev = pdev;
+	struct apple_pcie_port *port;
+
+	/* Find the root port this device is on */
+	port_pdev = pcie_find_root_port(pdev);
+
+	/* If finding the port itself, nothing to do */
+	if (WARN_ON(!port_pdev) || pdev == port_pdev)
+		return NULL;
+
+	list_for_each_entry(port, &pcie->ports, entry) {
+		if (port->idx == PCI_SLOT(port_pdev->devfn))
+			return port;
+	}
+
+	return NULL;
+}
+
+static int apple_pcie_add_device(struct apple_pcie_port *port,
+				 struct pci_dev *pdev)
+{
+	u32 sid, rid = PCI_DEVID(pdev->bus->number, pdev->devfn);
+	int idx, err;
+
+	dev_dbg(&pdev->dev, "added to bus %s, index %d\n",
+		pci_name(pdev->bus->self), port->idx);
+
+	err = of_map_id(port->pcie->dev->of_node, rid, "iommu-map",
+			"iommu-map-mask", NULL, &sid);
+	if (err)
+		return err;
+
+	mutex_lock(&port->pcie->lock);
+
+	idx = bitmap_find_free_region(port->sid_map, port->sid_map_sz, 0);
+	if (idx >= 0) {
+		apple_pcie_rid2sid_write(port, idx,
+					 PORT_RID2SID_VALID |
+					 (sid << PORT_RID2SID_SID_SHIFT) | rid);
+
+		dev_dbg(&pdev->dev, "mapping RID%x to SID%x (index %d)\n",
+			rid, sid, idx);
+	}
+
+	mutex_unlock(&port->pcie->lock);
+
+	return idx >= 0 ? 0 : -ENOSPC;
+}
+
+static void apple_pcie_release_device(struct apple_pcie_port *port,
+				      struct pci_dev *pdev)
+{
+	u32 rid = PCI_DEVID(pdev->bus->number, pdev->devfn);
+	int idx;
+
+	mutex_lock(&port->pcie->lock);
+
+	for_each_set_bit(idx, port->sid_map, port->sid_map_sz) {
+		u32 val;
+
+		val = readl_relaxed(port->base + PORT_RID2SID(idx));
+		if ((val & 0xffff) == rid) {
+			apple_pcie_rid2sid_write(port, idx, 0);
+			bitmap_release_region(port->sid_map, idx, 0);
+			dev_dbg(&pdev->dev, "Released %x (%d)\n", val, idx);
+			break;
+		}
+	}
+
+	mutex_unlock(&port->pcie->lock);
+}
+
+static int apple_pcie_bus_notifier(struct notifier_block *nb,
+				   unsigned long action,
+				   void *data)
+{
+	struct device *dev = data;
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct apple_pcie_port *port;
+	int err;
+
+	/*
+	 * This is a bit ugly. We assume that if we get notified for
+	 * any PCI device, we must be in charge of it, and that there
+	 * is no other PCI controller in the whole system. It probably
+	 * holds for now, but who knows for how long?
+	 */
+	port = apple_pcie_get_port(pdev);
+	if (!port)
+		return NOTIFY_DONE;
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		err = apple_pcie_add_device(port, pdev);
+		if (err)
+			return notifier_from_errno(err);
+		break;
+	case BUS_NOTIFY_DEL_DEVICE:
+		apple_pcie_release_device(port, pdev);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block apple_pcie_nb = {
+	.notifier_call = apple_pcie_bus_notifier,
+};
+
 static int apple_pcie_init(struct pci_config_window *cfg)
 {
 	struct device *dev = cfg->parent;
@@ -626,6 +769,9 @@ static int apple_pcie_init(struct pci_config_window *cfg)
 	if (IS_ERR(pcie->base))
 		return -ENODEV;
 
+	cfg->priv = pcie;
+	INIT_LIST_HEAD(&pcie->ports);
+
 	for_each_child_of_node(dev->of_node, of_port) {
 		ret = apple_pcie_setup_port(pcie, of_port);
 		if (ret) {
@@ -637,6 +783,21 @@ static int apple_pcie_init(struct pci_config_window *cfg)
 	return apple_msi_init(pcie);
 }
 
+static int apple_pcie_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = bus_register_notifier(&pci_bus_type, &apple_pcie_nb);
+	if (ret)
+		return ret;
+
+	ret = pci_host_common_probe(pdev);
+	if (ret)
+		bus_unregister_notifier(&pci_bus_type, &apple_pcie_nb);
+
+	return ret;
+}
+
 static const struct pci_ecam_ops apple_pcie_cfg_ecam_ops = {
 	.init		= apple_pcie_init,
 	.pci_ops	= {
@@ -653,7 +814,7 @@ static const struct of_device_id apple_pcie_of_match[] = {
 MODULE_DEVICE_TABLE(of, apple_pcie_of_match);
 
 static struct platform_driver apple_pcie_driver = {
-	.probe	= pci_host_common_probe,
+	.probe	= apple_pcie_probe,
 	.driver	= {
 		.name			= "pcie-apple",
 		.of_match_table		= apple_pcie_of_match,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up
  2021-09-22 20:54 ` [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up Marc Zyngier
@ 2021-09-22 21:08   ` Sven Peter
  2021-09-22 21:14     ` Rob Herring
  2021-09-22 21:24     ` Marc Zyngier
  2021-09-22 21:09   ` Rob Herring
  1 sibling, 2 replies; 17+ messages in thread
From: Sven Peter @ 2021-09-22 21:08 UTC (permalink / raw)
  To: Marc Zyngier, devicetree, linux-kernel, linux-pci
  Cc: Bjorn Helgaas, Rob Herring, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Hector Martin, Robin Murphy, kernel-team

Hi,


On Wed, Sep 22, 2021, at 22:54, Marc Zyngier wrote:
> From: Alyssa Rosenzweig <alyssa@rosenzweig.io>
>
[...]
> +
> +	/* 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;
> +
> +	rmw_set(PORT_APPCLK_EN, port + PORT_APPCLK);

I think this should be

    rmw_set(PORT_APPCLK_EN, port->base + PORT_APPCLK);

> +
> +	rmw_set(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();

I don't think this barrier is required.

> +
> +	writel_relaxed(PORT_LTSSMCTL_START, port->base + PORT_LTSSMCTL);
> +
> +	return 0;
> +}
> +
[...]


Looks good to me otherwise,

Reviewed-by: Sven Peter <sven@svenpeter.dev>


Thanks,


Sven

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up
  2021-09-22 20:54 ` [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up Marc Zyngier
  2021-09-22 21:08   ` Sven Peter
@ 2021-09-22 21:09   ` Rob Herring
  1 sibling, 0 replies; 17+ messages in thread
From: Rob Herring @ 2021-09-22 21:09 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: devicetree, linux-kernel, PCI, Bjorn Helgaas, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Alyssa Rosenzweig, Stan Skowronek,
	Mark Kettenis, Sven Peter, Hector Martin, Robin Murphy,
	Android Kernel Team

On Wed, Sep 22, 2021 at 3:55 PM Marc Zyngier <maz@kernel.org> 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>
> ---
>  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 ba3e2419e86a..18fc0ed9a7f6 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..2479a4168439
> --- /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>
> + */
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/kernel.h>
> +#include <linux/iopoll.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/msi.h>
> +#include <linux/of_irq.h>
> +#include <linux/pci-ecam.h>
> +
> +#define CORE_RC_PHYIF_CTL              0x00024
> +#define   CORE_RC_PHYIF_CTL_RUN                BIT(0)
> +#define CORE_RC_PHYIF_STAT             0x00028
> +#define   CORE_RC_PHYIF_STAT_REFCLK    BIT(4)
> +#define CORE_RC_CTL                    0x00050
> +#define   CORE_RC_CTL_RUN              BIT(0)
> +#define CORE_RC_STAT                   0x00058
> +#define   CORE_RC_STAT_READY           BIT(0)
> +#define CORE_FABRIC_STAT               0x04000
> +#define   CORE_FABRIC_STAT_MASK                0x001F001F
> +#define CORE_LANE_CFG(port)            (0x84000 + 0x4000 * (port))
> +#define   CORE_LANE_CFG_REFCLK0REQ     BIT(0)
> +#define   CORE_LANE_CFG_REFCLK1                BIT(1)
> +#define   CORE_LANE_CFG_REFCLK0ACK     BIT(2)
> +#define   CORE_LANE_CFG_REFCLKEN       (BIT(9) | BIT(10))
> +#define CORE_LANE_CTL(port)            (0x84004 + 0x4000 * (port))
> +#define   CORE_LANE_CTL_CFGACC         BIT(15)
> +
> +#define PORT_LTSSMCTL                  0x00080
> +#define   PORT_LTSSMCTL_START          BIT(0)
> +#define PORT_INTSTAT                   0x00100
> +#define   PORT_INT_TUNNEL_ERR          31
> +#define   PORT_INT_CPL_TIMEOUT         23
> +#define   PORT_INT_RID2SID_MAPERR      22
> +#define   PORT_INT_CPL_ABORT           21
> +#define   PORT_INT_MSI_BAD_DATA                19
> +#define   PORT_INT_MSI_ERR             18
> +#define   PORT_INT_REQADDR_GT32                17
> +#define   PORT_INT_AF_TIMEOUT          15
> +#define   PORT_INT_LINK_DOWN           14
> +#define   PORT_INT_LINK_UP             12
> +#define   PORT_INT_LINK_BWMGMT         11
> +#define   PORT_INT_AER_MASK            (15 << 4)
> +#define   PORT_INT_PORT_ERR            4
> +#define   PORT_INT_INTx(i)             i
> +#define   PORT_INT_INTx_MASK           15
> +#define PORT_INTMSK                    0x00104
> +#define PORT_INTMSKSET                 0x00108
> +#define PORT_INTMSKCLR                 0x0010c
> +#define PORT_MSICFG                    0x00124
> +#define   PORT_MSICFG_EN               BIT(0)
> +#define   PORT_MSICFG_L2MSINUM_SHIFT   4
> +#define PORT_MSIBASE                   0x00128
> +#define   PORT_MSIBASE_1_SHIFT         16
> +#define PORT_MSIADDR                   0x00168
> +#define PORT_LINKSTS                   0x00208
> +#define   PORT_LINKSTS_UP              BIT(0)
> +#define   PORT_LINKSTS_BUSY            BIT(2)
> +#define PORT_LINKCMDSTS                        0x00210
> +#define PORT_OUTS_NPREQS               0x00284
> +#define   PORT_OUTS_NPREQS_REQ         BIT(24)
> +#define   PORT_OUTS_NPREQS_CPL         BIT(16)
> +#define PORT_RXWR_FIFO                 0x00288
> +#define   PORT_RXWR_FIFO_HDR           GENMASK(15, 10)
> +#define   PORT_RXWR_FIFO_DATA          GENMASK(9, 0)
> +#define PORT_RXRD_FIFO                 0x0028C
> +#define   PORT_RXRD_FIFO_REQ           GENMASK(6, 0)
> +#define PORT_OUTS_CPLS                 0x00290
> +#define   PORT_OUTS_CPLS_SHRD          GENMASK(14, 8)
> +#define   PORT_OUTS_CPLS_WAIT          GENMASK(6, 0)
> +#define PORT_APPCLK                    0x00800
> +#define   PORT_APPCLK_EN               BIT(0)
> +#define   PORT_APPCLK_CGDIS            BIT(8)
> +#define PORT_STATUS                    0x00804
> +#define   PORT_STATUS_READY            BIT(0)
> +#define PORT_REFCLK                    0x00810
> +#define   PORT_REFCLK_EN               BIT(0)
> +#define   PORT_REFCLK_CGDIS            BIT(8)
> +#define PORT_PERST                     0x00814
> +#define   PORT_PERST_OFF               BIT(0)
> +#define PORT_RID2SID(i16)              (0x00828 + 4 * (i16))
> +#define   PORT_RID2SID_VALID           BIT(31)
> +#define   PORT_RID2SID_SID_SHIFT       16
> +#define   PORT_RID2SID_BUS_SHIFT       8
> +#define   PORT_RID2SID_DEV_SHIFT       3
> +#define   PORT_RID2SID_FUNC_SHIFT      0
> +#define PORT_OUTS_PREQS_HDR            0x00980
> +#define   PORT_OUTS_PREQS_HDR_MASK     GENMASK(9, 0)
> +#define PORT_OUTS_PREQS_DATA           0x00984
> +#define   PORT_OUTS_PREQS_DATA_MASK    GENMASK(15, 0)
> +#define PORT_TUNCTRL                   0x00988
> +#define   PORT_TUNCTRL_PERST_ON                BIT(0)
> +#define   PORT_TUNCTRL_PERST_ACK_REQ   BIT(1)
> +#define PORT_TUNSTAT                   0x0098c
> +#define   PORT_TUNSTAT_PERST_ON                BIT(0)
> +#define   PORT_TUNSTAT_PERST_ACK_PEND  BIT(1)
> +#define PORT_PREFMEM_ENABLE            0x00994
> +
> +struct apple_pcie {
> +       struct device           *dev;
> +       void __iomem            *base;
> +};
> +
> +struct apple_pcie_port {
> +       struct apple_pcie       *pcie;
> +       struct device_node      *np;
> +       void __iomem            *base;
> +       int                     idx;
> +};
> +
> +static void rmw_set(u32 set, void __iomem *addr)
> +{
> +       writel_relaxed(readl_relaxed(addr) | set, addr);
> +}
> +
> +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;
> +
> +       rmw_set(PORT_APPCLK_EN, port + PORT_APPCLK);
> +
> +       rmw_set(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();

I thought this was getting removed.

Otherwise,

Reviewed-by: Rob Herring <robh@kernel.org>

> +       writel_relaxed(PORT_LTSSMCTL_START, port->base + PORT_LTSSMCTL);
> +
> +       return 0;
> +}
> +
> +static int apple_pcie_init(struct pci_config_window *cfg)
> +{
> +       struct device *dev = cfg->parent;
> +       struct platform_device *platform = to_platform_device(dev);
> +       struct device_node *of_port;
> +       struct apple_pcie *pcie;
> +       int ret;
> +
> +       pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> +       if (!pcie)
> +               return -ENOMEM;
> +
> +       pcie->dev = dev;
> +
> +       mutex_init(&pcie->lock);
> +
> +       pcie->base = devm_platform_ioremap_resource(platform, 1);
> +
> +       if (IS_ERR(pcie->base))
> +               return -ENODEV;
> +
> +       for_each_child_of_node(dev->of_node, of_port) {
> +               ret = apple_pcie_setup_port(pcie, of_port);
> +               if (ret) {
> +                       dev_err(pcie->dev, "Port %pOF setup fail: %d\n", of_port, ret);
> +                       return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct pci_ecam_ops apple_pcie_cfg_ecam_ops = {
> +       .init           = apple_pcie_init,
> +       .pci_ops        = {
> +               .map_bus        = pci_ecam_map_bus,
> +               .read           = pci_generic_config_read,
> +               .write          = pci_generic_config_write,
> +       }
> +};
> +
> +static const struct of_device_id apple_pcie_of_match[] = {
> +       { .compatible = "apple,pcie", .data = &apple_pcie_cfg_ecam_ops },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, apple_pcie_of_match);
> +
> +static struct platform_driver apple_pcie_driver = {
> +       .probe  = pci_host_common_probe,
> +       .driver = {
> +               .name                   = "pcie-apple",
> +               .of_match_table         = apple_pcie_of_match,
> +               .suppress_bind_attrs    = true,
> +       },
> +};
> +module_platform_driver(apple_pcie_driver);
> +
> +MODULE_LICENSE("GPL v2");
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up
  2021-09-22 21:08   ` Sven Peter
@ 2021-09-22 21:14     ` Rob Herring
  2021-09-22 21:31       ` Marc Zyngier
  2021-09-22 21:24     ` Marc Zyngier
  1 sibling, 1 reply; 17+ messages in thread
From: Rob Herring @ 2021-09-22 21:14 UTC (permalink / raw)
  To: Sven Peter
  Cc: Marc Zyngier, devicetree, linux-kernel, PCI, Bjorn Helgaas,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Alyssa Rosenzweig,
	Stan Skowronek, Mark Kettenis, Hector Martin, Robin Murphy,
	Android Kernel Team

On Wed, Sep 22, 2021 at 4:08 PM Sven Peter <sven@svenpeter.dev> wrote:
>
> Hi,
>
>
> On Wed, Sep 22, 2021, at 22:54, Marc Zyngier wrote:
> > From: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> >
> [...]
> > +
> > +     /* 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;

Don't change error codes.

> > +
> > +     rmw_set(PORT_APPCLK_EN, port + PORT_APPCLK);
>
> I think this should be
>
>     rmw_set(PORT_APPCLK_EN, port->base + PORT_APPCLK);

Or just removed if this was tested and worked.

Rob

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up
  2021-09-22 21:08   ` Sven Peter
  2021-09-22 21:14     ` Rob Herring
@ 2021-09-22 21:24     ` Marc Zyngier
  1 sibling, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 21:24 UTC (permalink / raw)
  To: Sven Peter
  Cc: devicetree, linux-kernel, linux-pci, Bjorn Helgaas, Rob Herring,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Alyssa Rosenzweig,
	Stan Skowronek, Mark Kettenis, Hector Martin, Robin Murphy,
	kernel-team

On Wed, 22 Sep 2021 22:08:33 +0100,
"Sven Peter" <sven@svenpeter.dev> wrote:
> 
> Hi,
> 
> 
> On Wed, Sep 22, 2021, at 22:54, Marc Zyngier wrote:
> > From: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> >
> [...]
> > +
> > +	/* 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;
> > +
> > +	rmw_set(PORT_APPCLK_EN, port + PORT_APPCLK);
> 
> I think this should be
> 
>     rmw_set(PORT_APPCLK_EN, port->base + PORT_APPCLK);

Ouch. Well caught. I wonder how many of these I introduced...:-/

> 
> > +
> > +	rmw_set(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();
> 
> I don't think this barrier is required.

It really isn't, and I though I had removed it. I now wonder whether I
have pushed out the right branch, or messed up by moving from one
machine to another...

> > +
> > +	writel_relaxed(PORT_LTSSMCTL_START, port->base + PORT_LTSSMCTL);
> > +
> > +	return 0;
> > +}
> > +
> [...]
> 
> 
> Looks good to me otherwise,
> 
> Reviewed-by: Sven Peter <sven@svenpeter.dev>

Thanks. Hopefully I'll manage to post a non broken series next time...

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up
  2021-09-22 21:14     ` Rob Herring
@ 2021-09-22 21:31       ` Marc Zyngier
  0 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2021-09-22 21:31 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sven Peter, devicetree, linux-kernel, PCI, Bjorn Helgaas,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Alyssa Rosenzweig,
	Stan Skowronek, Mark Kettenis, Hector Martin, Robin Murphy,
	Android Kernel Team

On Wed, 22 Sep 2021 22:14:49 +0100,
Rob Herring <robh+dt@kernel.org> wrote:
> 
> On Wed, Sep 22, 2021 at 4:08 PM Sven Peter <sven@svenpeter.dev> wrote:
> >
> > Hi,
> >
> >
> > On Wed, Sep 22, 2021, at 22:54, Marc Zyngier wrote:
> > > From: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> > >
> > [...]
> > > +
> > > +     /* 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;
> 
> Don't change error codes.

Yup, good point. There is another instance of that when setting up a
port.

> 
> > > +
> > > +     rmw_set(PORT_APPCLK_EN, port + PORT_APPCLK);
> >
> > I think this should be
> >
> >     rmw_set(PORT_APPCLK_EN, port->base + PORT_APPCLK);
> 
> Or just removed if this was tested and worked.

That's probably the point where I find out that it only worked because
I was corrupting memory in bizarre ways. Oh well.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 00/10] PCI: Add support for Apple M1
  2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
                   ` (9 preceding siblings ...)
  2021-09-22 20:54 ` [PATCH v4 10/10] PCI: apple: Configure RID to SID mapper on device addition Marc Zyngier
@ 2021-09-22 23:32 ` Mark Kettenis
  10 siblings, 0 replies; 17+ messages in thread
From: Mark Kettenis @ 2021-09-22 23:32 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: devicetree, linux-kernel, linux-pci, bhelgaas, robh+dt,
	lorenzo.pieralisi, kw, alyssa, stan, kettenis, sven, marcan,
	Robin.Murphy, kernel-team

> From: Marc Zyngier <maz@kernel.org>
> Date: Wed, 22 Sep 2021 21:54:48 +0100
> 
> This is v4 of the series adding PCIe support for the M1 SoC. Not a lot
> has changed this time around, and most of what I was saying in [1] is
> still valid.
> 
> The most important change is that the driver now probes for the number
> of RID-SID mapping registers instead of assuming 64 entries. The rest
> is a bunch of limited cleanups and minor fixes.
> 
> This should now be in a state that makes it mergeable, although I
> expect that some of the clock bits may have to be adapted (I haven't
> followed the recent developments on that front).

The current understanding is that the M1 SoC really only has power
domains.  Fortunately power domains are handled automagically by the
Linux kernel (and U-Boot) so this driver doesn't have to worry about
this.

I already changed the 4/4 diff of my DT bindings series to add a
"power-domains" property instead of a "clocks" property.  So once
marcan's power manager driver lands everything should just work.  Some
coordination on the patch that changes the DT itself is probably
required.  But we could simply leave out the "power-domains" property
until the power manager driver lands as m1n1 currently already turns
on the power domain.

Cheers,

Mark

> As always, comments welcome.
> 
> [1] https://lore.kernel.org/r/20210913182550.264165-1-maz@kernel.org
> 
> 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           |  27 +
>  drivers/of/irq.c                     |  17 +-
>  drivers/pci/controller/Kconfig       |  17 +
>  drivers/pci/controller/Makefile      |   1 +
>  drivers/pci/controller/pcie-apple.c  | 826 +++++++++++++++++++++++++++
>  drivers/pci/of.c                     |  10 +-
>  include/linux/irqdomain.h            |   4 +
>  kernel/irq/irqdomain.c               |   6 +-
>  10 files changed, 935 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/pci/controller/pcie-apple.c
> 
> -- 
> 2.30.2
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-09-22 23:32 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 20:54 [PATCH v4 00/10] PCI: Add support for Apple M1 Marc Zyngier
2021-09-22 20:54 ` [PATCH v4 01/10] irqdomain: Make of_phandle_args_to_fwspec generally available Marc Zyngier
2021-09-22 20:54 ` [PATCH v4 02/10] of/irq: Allow matching of an interrupt-map local to an interrupt controller Marc Zyngier
2021-09-22 20:54 ` [PATCH v4 03/10] PCI: of: Allow matching of an interrupt-map local to a PCI device Marc Zyngier
2021-09-22 20:54 ` [PATCH v4 04/10] PCI: apple: Add initial hardware bring-up Marc Zyngier
2021-09-22 21:08   ` Sven Peter
2021-09-22 21:14     ` Rob Herring
2021-09-22 21:31       ` Marc Zyngier
2021-09-22 21:24     ` Marc Zyngier
2021-09-22 21:09   ` Rob Herring
2021-09-22 20:54 ` [PATCH v4 05/10] PCI: apple: Set up reference clocks when probing Marc Zyngier
2021-09-22 20:54 ` [PATCH v4 06/10] PCI: apple: Add INTx and per-port interrupt support Marc Zyngier
2021-09-22 20:54 ` [PATCH v4 07/10] arm64: apple: t8103: Add root port interrupt routing Marc Zyngier
2021-09-22 20:54 ` [PATCH v4 08/10] PCI: apple: Implement MSI support Marc Zyngier
2021-09-22 20:54 ` [PATCH v4 09/10] iommu/dart: Exclude MSI doorbell from PCIe device IOVA range Marc Zyngier
2021-09-22 20:54 ` [PATCH v4 10/10] PCI: apple: Configure RID to SID mapper on device addition Marc Zyngier
2021-09-22 23:32 ` [PATCH v4 00/10] PCI: Add support for Apple M1 Mark Kettenis

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).