[PATCH v2 0/3] PCI: apple: Integrate pwrctrl API

[PATCH v2 0/3] PCI: apple: Integrate pwrctrl API

[Yureka Lilian]

Hi all,

This series integrates the PCI pwrctrl framework into the Apple PCIe
host driver to provide standardized power management for PCI ports.

This allows powering on the WiFi, SD card reader, and USB xHCI on
Apple Silicon Macs through the pwrctrl framework before probing the ports.

The first patch in this series exposes the necessary APIs to create,
destroy, and power on/off pwrctrl devices for individual ports.
The second patch integrates these APIs into the Apple PCIe driver on a
per-port basis. The third patch adds device tree changes for t600x as an
example.

Originally, a custom solution for powering on the WiFi and SD card
reader was proposed[1], but we can now use the pci-pwrctrl framework
for this purpose.

Link[1]: https://lore.kernel.org/lkml/20220502093832.32778-4-marcan@marcan.st/
---
Changes in v2:
- Split the pwrctrl API integration to work on a per-port basis for
  consistency with the rest of the driver
- Correctly use ERR_PTR to convert the error value before printing

To: Bartosz Golaszewski <brgl@kernel.org>
To: Manivannan Sadhasivam <mani@kernel.org>
To: Bjorn Helgaas <bhelgaas@google.com>
To: Lorenzo Pieralisi <lpieralisi@kernel.org>
To: Krzysztof Wilczyński <kwilczynski@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Marc Zyngier <maz@kernel.org>
To: Sven Peter <sven@kernel.org>
To: Janne Grunau <j@jannau.net>
To: Neal Gompa <neal@gompa.dev>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: linux-pci@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: asahi@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org

---
Yureka Lilian (3):
      PCI/pwrctrl: Export per-device APIs
      PCI: apple: Integrate pwrctrl API
      arm64: dts: apple: t600x: Add PCIe pwren gpios

 arch/arm64/boot/dts/apple/t600x-die0.dtsi      |  4 ++
 arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi | 16 ++++++++
 arch/arm64/boot/dts/apple/t600x-j375.dtsi      | 27 ++++++++++++++
 drivers/pci/controller/Kconfig                 |  1 +
 drivers/pci/controller/pcie-apple.c            | 23 ++++++++++++
 drivers/pci/pwrctrl/core.c                     | 51 +++++++++++++++++++++++---
 include/linux/pci-pwrctrl.h                    |  4 ++
 7 files changed, 121 insertions(+), 5 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20260706-apple-pcie-pwren-0dd1ebdf1c4c

Best regards,
--  
Yureka Lilian <yureka@cyberchaos.dev>

[PATCH v2 1/3] PCI/pwrctrl: Export per-device APIs

[Yureka Lilian]

Export the `pci_pwrctrl_power_on_device()` and
`pci_pwrctrl_power_off_device()` functions to allow PCIe host drivers
to call manage power control on a per-port basis.

Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
---
 drivers/pci/pwrctrl/core.c  | 51 ++++++++++++++++++++++++++++++++++++++++-----
 include/linux/pci-pwrctrl.h |  4 ++++
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
index 0e2387cf4657..b59dbedf251b 100644
--- a/drivers/pci/pwrctrl/core.c
+++ b/drivers/pci/pwrctrl/core.c
@@ -191,7 +191,16 @@ static int __pci_pwrctrl_power_off_device(struct device *dev)
 	return pwrctrl->power_off(pwrctrl);
 }
 
-static void pci_pwrctrl_power_off_device(struct device_node *np)
+/**
+ * pci_pwrctrl_power_off_device - Power off pwrctrl devices
+ *
+ * @device_node: PCI port or device to operate on
+ *
+ * Recursively traverse all pwrctrl devices for the devicetree hierarchy
+ * below the specified PCI port or device and power them off in a depth
+ * first manner.
+ */
+void pci_pwrctrl_power_off_device(struct device_node *np)
 {
 	struct platform_device *pdev;
 	int ret;
@@ -216,6 +225,7 @@ static void pci_pwrctrl_power_off_device(struct device_node *np)
 
 	platform_device_put(pdev);
 }
+EXPORT_SYMBOL_GPL(pci_pwrctrl_power_off_device);
 
 /**
  * pci_pwrctrl_power_off_devices - Power off pwrctrl devices
@@ -245,11 +255,21 @@ static int __pci_pwrctrl_power_on_device(struct device *dev)
 	return pwrctrl->power_on(pwrctrl);
 }
 
-/*
+/**
+ * pci_pwrctrl_power_on_device - Power on pwrctrl devices
+ *
+ * @device_node: PCI port or device to operate on
+ *
  * Power on the devices in a depth first manner. Before powering on the device,
  * make sure its driver is bound.
+ * Recursively traverse all pwrctrl devices for the devicetree hierarchy
+ * below the specified PCI port or device and power them on in a depth
+ * first manner.  Before powering on the device, make sure its driver is bound.
+ *
+ * Return: 0 on success, -EPROBE_DEFER if any pwrctrl driver is not bound, an
+ * appropriate error code otherwise.
  */
-static int pci_pwrctrl_power_on_device(struct device_node *np)
+int pci_pwrctrl_power_on_device(struct device_node *np)
 {
 	struct platform_device *pdev;
 	struct device_node *child;
@@ -296,6 +316,7 @@ static int pci_pwrctrl_power_on_device(struct device_node *np)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(pci_pwrctrl_power_on_device);
 
 /**
  * pci_pwrctrl_power_on_devices - Power on pwrctrl devices
@@ -335,7 +356,17 @@ int pci_pwrctrl_power_on_devices(struct device *parent)
 }
 EXPORT_SYMBOL_GPL(pci_pwrctrl_power_on_devices);
 
-static int pci_pwrctrl_create_device(struct device_node *np,
+/**
+ * pci_pwrctrl_create_device - Create pwrctrl devices
+ *
+ * @device_node: PCI port or device to operate on
+ *
+ * Recursively create pwrctrl devices for the devicetree hierarchy below
+ * the specified PCI port or device in a depth first manner.
+ *
+ * Return: 0 on success, negative error number on error.
+ */
+int pci_pwrctrl_create_device(struct device_node *np,
 				     struct device *parent)
 {
 	struct platform_device *pdev;
@@ -368,6 +399,7 @@ static int pci_pwrctrl_create_device(struct device_node *np,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(pci_pwrctrl_create_device);
 
 /**
  * pci_pwrctrl_create_devices - Create pwrctrl devices
@@ -396,7 +428,15 @@ int pci_pwrctrl_create_devices(struct device *parent)
 }
 EXPORT_SYMBOL_GPL(pci_pwrctrl_create_devices);
 
-static void pci_pwrctrl_destroy_device(struct device_node *np)
+/**
+ * pci_pwrctrl_destroy_device - Destroy pwrctrl devices
+ *
+ * @device_node: PCI port or device to operate on
+ *
+ * Recursively destroy pwrctrl devices for the devicetree hierarchy below
+ * the specified PCI port or device in a depth first manner.
+ */
+void pci_pwrctrl_destroy_device(struct device_node *np)
 {
 	struct platform_device *pdev;
 
@@ -415,6 +455,7 @@ static void pci_pwrctrl_destroy_device(struct device_node *np)
 
 	of_node_clear_flag(np, OF_POPULATED);
 }
+EXPORT_SYMBOL_GPL(pci_pwrctrl_destroy_device);
 
 /**
  * pci_pwrctrl_destroy_devices - Destroy pwrctrl devices
diff --git a/include/linux/pci-pwrctrl.h b/include/linux/pci-pwrctrl.h
index 1192a2527521..f27be6489b50 100644
--- a/include/linux/pci-pwrctrl.h
+++ b/include/linux/pci-pwrctrl.h
@@ -59,6 +59,10 @@ int pci_pwrctrl_create_devices(struct device *parent);
 void pci_pwrctrl_destroy_devices(struct device *parent);
 int pci_pwrctrl_power_on_devices(struct device *parent);
 void pci_pwrctrl_power_off_devices(struct device *parent);
+int pci_pwrctrl_create_device(struct device_node *np, struct device *parent);
+void pci_pwrctrl_destroy_device(struct device_node *np);
+int pci_pwrctrl_power_on_device(struct device_node *np);
+void pci_pwrctrl_power_off_device(struct device_node *np);
 #else
 static inline int pci_pwrctrl_create_devices(struct device *parent) { return 0; }
 static void pci_pwrctrl_destroy_devices(struct device *parent) { }

-- 
2.56.0

[PATCH v2 2/3] PCI: apple: Integrate pwrctrl API

[Yureka Lilian]

Integrate the PCI pwrctrl framework into the Apple PCIe host driver to
provide standardized power management for PCI devices.

Notably, this allows enabling powering on the WiFi, SD card reader on
various Macs by means of the pwrctrl framework before probing the ports.

Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
---
 drivers/pci/controller/Kconfig      |  1 +
 drivers/pci/controller/pcie-apple.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index d246bbe37948..791892a6c0c6 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -46,6 +46,7 @@ config PCIE_APPLE
 	depends on OF
 	depends on PCI_MSI
 	select PCI_HOST_COMMON
+	select PCI_PWRCTRL_GENERIC
 	select IRQ_MSI_LIB
 	help
 	  Say Y here if you want to enable PCIe controller support on Apple
diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index c2cffc0659f4..2d32092e2122 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -30,6 +30,7 @@
 #include <linux/msi.h>
 #include <linux/of_irq.h>
 #include <linux/pci-ecam.h>
+#include <linux/pci-pwrctrl.h>
 
 #include "pci-host-common.h"
 
@@ -825,6 +826,28 @@ static int apple_pcie_init(struct pci_config_window *cfg)
 	if (WARN_ON(!pcie))
 		return -ENOENT;
 
+	// Ensure all ports can be powered on before attempting initialization
+	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
+		ret = pci_pwrctrl_create_device(of_port, pcie->dev);
+		if (ret) {
+			dev_err(pcie->dev, "Failed to create pwrctrl device: %pe\n", ERR_PTR(ret));
+			return ret;
+		}
+
+		ret = pci_pwrctrl_power_on_device(of_port);
+		if (ret) {
+			if (ret != -EPROBE_DEFER) {
+				dev_err(pcie->dev,
+					"Failed to power on device: %pe\n",
+					ERR_PTR(ret));
+				for_each_available_child_of_node_scoped(dev->of_node, of_port) {
+					pci_pwrctrl_destroy_device(of_port);
+				}
+			}
+			return ret;
+		}
+	}
+
 	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
 		ret = apple_pcie_setup_port(pcie, of_port);
 		if (ret) {

-- 
2.56.0

[PATCH v2 3/3] arm64: dts: apple: t600x: Add PCIe pwren gpios

[Yureka Lilian]

For the MacBook Pro and Mac Studio devices using the M1 Pro/Max/Ultra
SoCs, add the appropriate fixed regulators for enabling the peripherals
connected to the PCIe ports (WiFi, SD card reader and USB xHCI), which
are controlled by GPIOs connected to the SMC.
Set the regulators as the power-supply of the respective PCIe port, so
that the endpoints are powered on before probing the ports.

Add the 'pciclass,0604' compatible string to the PCIe bridge nodes in
the SoC to allow the pci-pwrctrl-generic driver to bind to them.

Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
---
 arch/arm64/boot/dts/apple/t600x-die0.dtsi      |  4 ++++
 arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi | 16 +++++++++++++++
 arch/arm64/boot/dts/apple/t600x-j375.dtsi      | 27 ++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/arch/arm64/boot/dts/apple/t600x-die0.dtsi b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
index 122a566e75d1..635635c4bfbd 100644
--- a/arch/arm64/boot/dts/apple/t600x-die0.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
@@ -460,6 +460,7 @@ pcie0: pcie@590000000 {
 		pinctrl-names = "default";
 
 		port00: pci@0,0 {
+			compatible = "pciclass,0604";
 			device_type = "pci";
 			reg = <0x0 0x0 0x0 0x0 0x0>;
 			reset-gpios = <&pinctrl_ap 4 GPIO_ACTIVE_LOW>;
@@ -479,6 +480,7 @@ port00: pci@0,0 {
 		};
 
 		port01: pci@1,0 {
+			compatible = "pciclass,0604";
 			device_type = "pci";
 			reg = <0x800 0x0 0x0 0x0 0x0>;
 			reset-gpios = <&pinctrl_ap 5 GPIO_ACTIVE_LOW>;
@@ -498,6 +500,7 @@ port01: pci@1,0 {
 		};
 
 		port02: pci@2,0 {
+			compatible = "pciclass,0604";
 			device_type = "pci";
 			reg = <0x1000 0x0 0x0 0x0 0x0>;
 			reset-gpios = <&pinctrl_ap 6 GPIO_ACTIVE_LOW>;
@@ -518,6 +521,7 @@ port02: pci@2,0 {
 		};
 
 		port03: pci@3,0 {
+			compatible = "pciclass,0604";
 			device_type = "pci";
 			reg = <0x1800 0x0 0x0 0x0 0x0>;
 			reset-gpios = <&pinctrl_ap 7 GPIO_ACTIVE_LOW>;
diff --git a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
index caded1636f3d..4f9dccf839ad 100644
--- a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
@@ -55,6 +55,20 @@ led-0 {
 			default-state = "keep";
 		};
 	};
+
+	vreg_pcie0_port0: regulator-pcie0-port0 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwren-pcie0-port0";
+		gpio = <&smc_gpio 13 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vreg_pcie0_port1: regulator-pcie0-port1 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwren-pcie0-port1";
+		gpio = <&smc_gpio 26 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
 };
 
 &serial0 {
@@ -176,6 +190,7 @@ &nco_clkref {
 /* PCIe devices */
 &port00 {
 	/* WLAN */
+	power-supply = <&vreg_pcie0_port0>;
 	bus-range = <1 1>;
 	wifi0: wifi@0,0 {
 		compatible = "pci14e4,4433";
@@ -195,6 +210,7 @@ bluetooth0: bluetooth@0,1 {
 
 &port01 {
 	/* SD card reader */
+	power-supply = <&vreg_pcie0_port1>;
 	bus-range = <2 2>;
 	sdhci0: mmc@0,0 {
 		compatible = "pci17a0,9755";
diff --git a/arch/arm64/boot/dts/apple/t600x-j375.dtsi b/arch/arm64/boot/dts/apple/t600x-j375.dtsi
index 8a1494949e4c..412b2fe109be 100644
--- a/arch/arm64/boot/dts/apple/t600x-j375.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-j375.dtsi
@@ -42,6 +42,30 @@ memory@10000000000 {
 		device_type = "memory";
 		reg = <0x100 0 0x2 0>; /* To be filled by loader */
 	};
+
+	/* WLAN/BT */
+	vreg_pcie0_port0: regulator-pcie0-port0 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwren-pcie0-port0";
+		gpio = <&smc_gpio 13 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	/* SD card reader */
+	vreg_pcie0_port1: regulator-pcie0-port1 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwren-pcie0-port1";
+		gpio = <&smc_gpio 26 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	/* USB xHCI */
+	vreg_pcie0_port3: regulator-pcie0-port3 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwren-pcie0-port3";
+		gpio = <&smc_gpio 20 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
 };
 
 &serial0 {
@@ -356,6 +380,7 @@ &nco_clkref {
 /* PCIe devices */
 &port00 {
 	/* WLAN */
+	power-supply = <&vreg_pcie0_port0>;
 	bus-range = <1 1>;
 	wifi0: wifi@0,0 {
 		compatible = "pci14e4,4433";
@@ -375,6 +400,7 @@ bluetooth0: bluetooth@0,1 {
 
 &port01 {
 	/* SD card reader */
+	power-supply = <&vreg_pcie0_port1>;
 	bus-range = <2 2>;
 	sdhci0: mmc@0,0 {
 		compatible = "pci17a0,9755";
@@ -397,6 +423,7 @@ ethernet0: ethernet@0,0 {
 
 &port03 {
 	/* USB xHCI */
+	power-supply = <&vreg_pcie0_port3>;
 	bus-range = <4 4>;
 	status = "okay";
 };

-- 
2.56.0

Re: [PATCH v2 1/3] PCI/pwrctrl: Export per-device APIs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] PCI/pwrctrl: leak of child devices on error in pci_pwrctrl_create_device()
- [Low] PCI/pwrctrl: missing !CONFIG_PCI_PWRCTRL stubs and device_node declaration
- [Low] PCI/pwrctrl: invalid kernel-doc annotations and duplicated comment in core.c
- [Low] PCI/pwrctrl: commit message omits exported create and destroy device APIs
--

commit 99f4514cfa7c5abc8881197d44fe3f03f5d6e806
Author: Yureka Lilian <yureka@cyberchaos.dev>

PCI/pwrctrl: Export per-device APIs

This commit exports pci_pwrctrl_power_on_device(),
pci_pwrctrl_power_off_device(), pci_pwrctrl_create_device(), and
pci_pwrctrl_destroy_device() for per-port power control management in PCIe
host controller drivers.

>     PCI/pwrctrl: Export per-device APIs
>     
>     Export the `pci_pwrctrl_power_on_device()` and
>     `pci_pwrctrl_power_off_device()` functions to allow PCIe host drivers
>     to call manage power control on a per-port basis.
>     
>     Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>

[Severity: Low]
This isn't a bug, but the patch also exports pci_pwrctrl_create_device()
and pci_pwrctrl_destroy_device(). Should the commit message mention those
two functions as well?

> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 0e2387cf46577..b59dbedf251bc 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -191,7 +191,16 @@ static int __pci_pwrctrl_power_off_device(struct device *dev)
>  	return pwrctrl->power_off(pwrctrl);
>  }
>  
> -static void pci_pwrctrl_power_off_device(struct device_node *np)
> +/**
> + * pci_pwrctrl_power_off_device - Power off pwrctrl devices
> + *
> + * @device_node: PCI port or device to operate on
> + *
> + * Recursively traverse all pwrctrl devices for the devicetree hierarchy
> + * below the specified PCI port or device and power them off in a depth
> + * first manner.
> + */
> +void pci_pwrctrl_power_off_device(struct device_node *np)

[ ... ]

> @@ -245,11 +255,21 @@ static int __pci_pwrctrl_power_on_device(struct device *dev)
>  	return pwrctrl->power_on(pwrctrl);
>  }
>  
> -/*
> +/**
> + * pci_pwrctrl_power_on_device - Power on pwrctrl devices
> + *
> + * @device_node: PCI port or device to operate on
> + *
>   * Power on the devices in a depth first manner. Before powering on the device,
>   * make sure its driver is bound.
> + * Recursively traverse all pwrctrl devices for the devicetree hierarchy
> + * below the specified PCI port or device and power them on in a depth
> + * first manner.  Before powering on the device, make sure its driver is bound.
> + *
> + * Return: 0 on success, -EPROBE_DEFER if any pwrctrl driver is not bound, an
> + * appropriate error code otherwise.
>   */
> -static int pci_pwrctrl_power_on_device(struct device_node *np)
> +int pci_pwrctrl_power_on_device(struct device_node *np)

[Severity: Low]
In the kernel-doc comments for pci_pwrctrl_power_off_device(),
pci_pwrctrl_power_on_device(), pci_pwrctrl_create_device(), and
pci_pwrctrl_destroy_device(), the parameter is documented as @device_node
rather than @np, and pci_pwrctrl_create_device() omits @parent.

Also, in pci_pwrctrl_power_on_device(), the previous two-sentence comment
was left in place right above the new description that repeats the same
text. Could these kernel-doc blocks be cleaned up to avoid kernel-doc
warnings?

[ ... ]

> @@ -335,7 +356,17 @@ int pci_pwrctrl_power_on_devices(struct device *parent)
>  }
>  EXPORT_SYMBOL_GPL(pci_pwrctrl_power_on_devices);
>  
> -static int pci_pwrctrl_create_device(struct device_node *np,
> +/**
> + * pci_pwrctrl_create_device - Create pwrctrl devices
> + *
> + * @device_node: PCI port or device to operate on
> + *
> + * Recursively create pwrctrl devices for the devicetree hierarchy below
> + * the specified PCI port or device in a depth first manner.
> + *
> + * Return: 0 on success, negative error number on error.
> + */
> +int pci_pwrctrl_create_device(struct device_node *np,
>  				     struct device *parent)
>  {
>  	struct platform_device *pdev;
> @@ -368,6 +399,7 @@ static int pci_pwrctrl_create_device(struct device_node *np,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(pci_pwrctrl_create_device);

[Severity: Medium]
Does pci_pwrctrl_create_device() leak already-created child platform
devices on failure?

Inside pci_pwrctrl_create_device():

	for_each_available_child_of_node_scoped(np, child) {
		ret = pci_pwrctrl_create_device(child, parent);
		if (ret)
			return ret;
	}
	...
	pdev = of_platform_device_create(np, NULL, parent);
	if (!pdev) {
		dev_err(parent, "Failed to create pwrctrl device for node: %s\n", np->name);
		return -EINVAL;
	}

When pci_pwrctrl_create_device() was only called from
pci_pwrctrl_create_devices(), the caller cleaned up partially created
devices on error by calling pci_pwrctrl_destroy_devices(parent).

Now that pci_pwrctrl_create_device() is exported for direct per-port calls,
if device creation fails for a later sibling child or for np itself, any
child platform devices already created during the call remain registered
with OF_POPULATED set. Should pci_pwrctrl_create_device() unwind and
destroy the already-created children on error, similar to the err_power_off
rollback in pci_pwrctrl_power_on_device()?

[ ... ]

> diff --git a/include/linux/pci-pwrctrl.h b/include/linux/pci-pwrctrl.h
> index 1192a2527521d..f27be6489b50f 100644
> --- a/include/linux/pci-pwrctrl.h
> +++ b/include/linux/pci-pwrctrl.h
> @@ -59,6 +59,10 @@ int pci_pwrctrl_create_devices(struct device *parent);
>  void pci_pwrctrl_destroy_devices(struct device *parent);
>  int pci_pwrctrl_power_on_devices(struct device *parent);
>  void pci_pwrctrl_power_off_devices(struct device *parent);
> +int pci_pwrctrl_create_device(struct device_node *np, struct device *parent);
> +void pci_pwrctrl_destroy_device(struct device_node *np);
> +int pci_pwrctrl_power_on_device(struct device_node *np);
> +void pci_pwrctrl_power_off_device(struct device_node *np);
>  #else
>  static inline int pci_pwrctrl_create_devices(struct device *parent) { return 0; }
>  static void pci_pwrctrl_destroy_devices(struct device *parent) { }

[Severity: Low]
Will drivers calling these new functions fail to build when
CONFIG_PCI_PWRCTRL is disabled?

The declarations are added under #if IS_ENABLED(CONFIG_PCI_PWRCTRL), but
there are no corresponding static inline stub definitions in the #else
block. In addition, include/linux/pci-pwrctrl.h forward-declares struct
device and struct device_link, but does not forward-declare struct
device_node, which can trigger compiler warnings if this header is included
before linux/of.h.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-apple-pcie-pwren-v2-0-216a4066965f@cyberchaos.dev?part=1

Re: [PATCH v2 2/3] PCI: apple: Integrate pwrctrl API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] PCI: apple: unbalanced pwrctrl power and device cleanup in apple_pcie_init()
--

commit 3ba5f942a54bed93131c2e3157428ab3600ebf03
Author: Yureka Lilian <yureka@cyberchaos.dev>

PCI: apple: Integrate pwrctrl API

This commit integrates the PCI pwrctrl API into the Apple PCIe host
controller driver so child port devices are created and powered on before
setting up the ports in apple_pcie_init().

> diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
> index c2cffc0659f44..2d32092e2122b 100644
> --- a/drivers/pci/controller/pcie-apple.c
> +++ b/drivers/pci/controller/pcie-apple.c

[ ... ]

> @@ -825,6 +826,28 @@ static int apple_pcie_init(struct pci_config_window *cfg)
>  	if (WARN_ON(!pcie))
>  		return -ENOENT;
>  
> +	// Ensure all ports can be powered on before attempting initialization
> +	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> +		ret = pci_pwrctrl_create_device(of_port, pcie->dev);
> +		if (ret) {
> +			dev_err(pcie->dev, "Failed to create pwrctrl device: %pe\n", ERR_PTR(ret));
> +			return ret;
> +		}
> +
> +		ret = pci_pwrctrl_power_on_device(of_port);
> +		if (ret) {
> +			if (ret != -EPROBE_DEFER) {
> +				dev_err(pcie->dev,
> +					"Failed to power on device: %pe\n",
> +					ERR_PTR(ret));
> +				for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> +					pci_pwrctrl_destroy_device(of_port);
> +				}
> +			}
> +			return ret;
> +		}
> +	}
> +
>  	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
>  		ret = apple_pcie_setup_port(pcie, of_port);
>  		if (ret) {

[Severity: Medium]
Can this loop in apple_pcie_init() leave pwrctrl devices powered on or leaked
when an error or probe deferral occurs on a multi-port controller?

If an earlier port succeeds in both pci_pwrctrl_create_device() and
pci_pwrctrl_power_on_device(), and a subsequent port fails in:

1. pci_pwrctrl_create_device(): apple_pcie_init() returns immediately without
   powering off or destroying the pwrctrl devices created and powered on in
   previous iterations.

2. pci_pwrctrl_power_on_device() with -EPROBE_DEFER:
   pci_pwrctrl_power_on_device() only unwinds children of the failing port
   node, leaving earlier sibling ports powered on. When deferred probing
   retries apple_pcie_init(), pci_pwrctrl_power_on_device() is called again
   on the earlier ports, enabling their regulators and clocks a second time
   and leaking enable reference counts on every retry.

3. pci_pwrctrl_power_on_device() with a non-deferral error: the error path
   calls pci_pwrctrl_destroy_device() without first calling
   pci_pwrctrl_power_off_device() on the ports that were already powered on,
   which unregisters active pwrctrl platform devices and frees regulators
   while they are still enabled. The inner
   for_each_available_child_of_node_scoped(dev->of_node, of_port) loop also
   redeclares of_port and shadows the outer loop variable (-Wshadow).

4. apple_pcie_setup_port() in the second loop: if port setup fails or returns
   -EPROBE_DEFER (for example from devm_fwnode_gpiod_get()), apple_pcie_init()
   returns without powering off the pwrctrl devices enabled in the first loop
   or destroying them on non-deferral errors.

Would it be cleaner to use the controller-level helpers
pci_pwrctrl_create_devices(), pci_pwrctrl_power_on_devices(),
pci_pwrctrl_power_off_devices(), and pci_pwrctrl_destroy_devices() with goto
error unwinding so all ports are balanced properly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-apple-pcie-pwren-v2-0-216a4066965f@cyberchaos.dev?part=2

Re: [PATCH v2 3/3] arm64: dts: apple: t600x: Add PCIe pwren gpios

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] arm64: dts: apple: t600x PCIe regulators break PCIe power on t602x boards
- [Medium] PCI: apple: missing pwrctrl power-off and cleanup on error in apple_pcie_init()
- [Low] arm64: dts: apple: undocumented power-supply property on PCIe bridge nodes
- [Low] PCI/pwrctrl: missing !CONFIG_PCI_PWRCTRL stubs for per-device pwrctrl APIs
- [Low] PCI/pwrctrl: mismatched kernel-doc parameter names in drivers/pci/pwrctrl/core.c
--

commit fc220e60fe2546bbee22affe9598f38be84ac7dc
Author: Yureka Lilian <yureka@cyberchaos.dev>

arm64: dts: apple: t600x: Add PCIe pwren gpios

This series exports per-device PCI power control helpers, integrates them into
the Apple PCIe controller driver, and adds SMC GPIO-controlled fixed
regulators for PCIe root ports on Apple T600x devicetrees.

> diff --git a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
> index caded1636f3d2..4f9dccf839ad0 100644
> --- a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
> +++ b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
> @@ -55,6 +55,20 @@ led-0 {
>  			default-state = "keep";
>  		};
>  	};
> +
> +	vreg_pcie0_port0: regulator-pcie0-port0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwren-pcie0-port0";
> +		gpio = <&smc_gpio 13 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	vreg_pcie0_port1: regulator-pcie0-port1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwren-pcie0-port1";
> +		gpio = <&smc_gpio 26 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
>  };

[Severity: High]
Will adding these regulators to t600x-j314-j316.dtsi and t600x-j375.dtsi cut
power to PCIe endpoints on T602x systems?

Both files are included by the T602x board files:

arch/arm64/boot/dts/apple/t602x-j414-j416.dtsi:
    /*
     * These models are essentially identical to the previous generation, other than
     * the GPIO indices.
     */

    #include "t600x-j314-j316.dtsi"

arch/arm64/boot/dts/apple/t602x-j474-j475.dtsi:
    /*
     * These models are very similar to the previous generation Mac Studio, other
     * than GPIO indices.
     */

    #include "t600x-j375.dtsi"

Because vreg_pcie0_port0, vreg_pcie0_port1, and vreg_pcie0_port3 do not set
regulator-boot-on, reg_fixed_voltage_probe() requests each SMC GPIO with
GPIOD_OUT_LOW and immediately drives the pin low via macsmc_gpio_set().

However, port00 through port03 in arch/arm64/boot/dts/apple/t602x-die0.dtsi do
not have compatible = "pciclass,0604":

arch/arm64/boot/dts/apple/t602x-die0.dtsi:
		port00: pci@0,0 {
			device_type = "pci";
			reg = <0x0 0x0 0x0 0x0 0x0>;
			reset-gpios = <&pinctrl_ap 4 GPIO_ACTIVE_LOW>;

As a result, pci_pwrctrl_is_required() returns false on T602x, no pwrctrl
device binds to re-enable the regulators after reg_fixed_voltage_probe() turns
them off, the T602x files note that GPIO indices differ from T600x, and
arch/arm64/boot/dts/apple/t6020-j474s.dts has an unused port01 where sdhci0 is
deleted (/delete-node/ &sdhci0;) while vreg_pcie0_port1 still claims smc_gpio
26.

[ ... ]

> @@ -176,6 +190,7 @@ &nco_clkref {
>  /* PCIe devices */
>  &port00 {
>  	/* WLAN */
> +	power-supply = <&vreg_pcie0_port0>;
>  	bus-range = <1 1>;
>  	wifi0: wifi@0,0 {
>  		compatible = "pci14e4,4433";
> @@ -195,6 +210,7 @@ bluetooth0: bluetooth@0,1 {
>  
>  &port01 {
>  	/* SD card reader */
> +	power-supply = <&vreg_pcie0_port1>;
>  	bus-range = <2 2>;
>  	sdhci0: mmc@0,0 {
>  		compatible = "pci17a0,9755";

[Severity: Low]
Should port00 and port01 here (and port00, port01, and port03 in
arch/arm64/boot/dts/apple/t600x-j375.dtsi) use a documented PCI slot supply
property such as vpcie3v3-supply instead of power-supply?

Documentation/devicetree/bindings/pci/apple,pcie.yaml sets
unevaluatedProperties: false and references /schemas/pci/pci-host-bridge.yaml#
for child bridge nodes, where power-supply is not a documented property and
causes dtbs_check warnings, whereas vpcie3v3-supply is defined by the PCI slot
schema and consumed by pci-pwrctrl-slot via of_pci_supply_present() and
of_regulator_bulk_get_all().

[ ... ]

> diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
> index c2cffc0659f44..2d32092e2122b 100644
> --- a/drivers/pci/controller/pcie-apple.c
> +++ b/drivers/pci/controller/pcie-apple.c
> @@ -30,6 +30,7 @@
>  #include <linux/msi.h>
>  #include <linux/of_irq.h>
>  #include <linux/pci-ecam.h>
> +#include <linux/pci-pwrctrl.h>
>  
>  #include "pci-host-common.h"
>  
> @@ -825,6 +826,28 @@ static int apple_pcie_init(struct pci_config_window *cfg)
>  	if (WARN_ON(!pcie))
>  		return -ENOENT;
>  
> +	// Ensure all ports can be powered on before attempting initialization
> +	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> +		ret = pci_pwrctrl_create_device(of_port, pcie->dev);
> +		if (ret) {
> +			dev_err(pcie->dev, "Failed to create pwrctrl device: %pe\n", ERR_PTR(ret));
> +			return ret;
> +		}
> +
> +		ret = pci_pwrctrl_power_on_device(of_port);
> +		if (ret) {
> +			if (ret != -EPROBE_DEFER) {
> +				dev_err(pcie->dev,
> +					"Failed to power on device: %pe\n",
> +					ERR_PTR(ret));
> +				for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> +					pci_pwrctrl_destroy_device(of_port);
> +				}
> +			}
> +			return ret;
> +		}
> +	}
> +
>  	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
>  		ret = apple_pcie_setup_port(pcie, of_port);
>  		if (ret) {

[Severity: Medium]
What happens to already-enabled port regulators if a subsequent port fails or
defers in apple_pcie_init()?

Once an earlier port succeeds in pci_pwrctrl_power_on_device(), its regulator
has been enabled via slot_pwrctrl_power_on().

1. If a later port returns -EPROBE_DEFER from pci_pwrctrl_power_on_device(),
   apple_pcie_init() returns without calling pci_pwrctrl_power_off_device() on
   the ports that were already powered on. On the next probe retry,
   pci_pwrctrl_power_on_device() will enable those regulators a second time,
   leaking regulator enable reference counts.

2. If pci_pwrctrl_power_on_device() fails with an error other than
   -EPROBE_DEFER, apple_pcie_init() calls pci_pwrctrl_destroy_device() on all
   ports without calling pci_pwrctrl_power_off_device() first, which unbinds
   the slot pwrctrl device and runs devm_slot_pwrctrl_release() ->
   regulator_bulk_free() while the regulator is still enabled, triggering
   WARN_ON(regulator->enable_count) in _regulator_put().

3. If pci_pwrctrl_create_device() fails on a later port or
   apple_pcie_setup_port() fails in the second loop, the previously created
   and powered-on pwrctrl devices are neither powered off nor destroyed.

Could apple_pcie_init() use pci_pwrctrl_create_devices() and
pci_pwrctrl_power_on_devices() (which already unwind previously powered-on or
created child nodes on error), and call pci_pwrctrl_power_off_devices() and
pci_pwrctrl_destroy_devices() if apple_pcie_setup_port() fails?

> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 0e2387cf46577..b59dbedf251bc 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -191,7 +191,16 @@ static int __pci_pwrctrl_power_off_device(struct device *dev)
>  	return pwrctrl->power_off(pwrctrl);
>  }
>  
> -static void pci_pwrctrl_power_off_device(struct device_node *np)
> +/**
> + * pci_pwrctrl_power_off_device - Power off pwrctrl devices
> + *
> + * @device_node: PCI port or device to operate on
> + *
> + * Recursively traverse all pwrctrl devices for the devicetree hierarchy
> + * below the specified PCI port or device and power them off in a depth
> + * first manner.
> + */
> +void pci_pwrctrl_power_off_device(struct device_node *np)

[ ... ]

> @@ -245,11 +255,21 @@ static int __pci_pwrctrl_power_on_device(struct device *dev)
>  	return pwrctrl->power_on(pwrctrl);
>  }
>  
> -/*
> +/**
> + * pci_pwrctrl_power_on_device - Power on pwrctrl devices
> + *
> + * @device_node: PCI port or device to operate on
> + *
>   * Power on the devices in a depth first manner. Before powering on the device,
>   * make sure its driver is bound.
> + * Recursively traverse all pwrctrl devices for the devicetree hierarchy
> + * below the specified PCI port or device and power them on in a depth
> + * first manner.  Before powering on the device, make sure its driver is bound.
> + *
> + * Return: 0 on success, -EPROBE_DEFER if any pwrctrl driver is not bound, an
> + * appropriate error code otherwise.
>   */
> -static int pci_pwrctrl_power_on_device(struct device_node *np)
> +int pci_pwrctrl_power_on_device(struct device_node *np)

[ ... ]

> @@ -335,7 +356,17 @@ int pci_pwrctrl_power_on_devices(struct device *parent)
>  }
>  EXPORT_SYMBOL_GPL(pci_pwrctrl_power_on_devices);
>  
> -static int pci_pwrctrl_create_device(struct device_node *np,
> +/**
> + * pci_pwrctrl_create_device - Create pwrctrl devices
> + *
> + * @device_node: PCI port or device to operate on
> + *
> + * Recursively create pwrctrl devices for the devicetree hierarchy below
> + * the specified PCI port or device in a depth first manner.
> + *
> + * Return: 0 on success, negative error number on error.
> + */
> +int pci_pwrctrl_create_device(struct device_node *np,
>  				     struct device *parent)

[ ... ]

> @@ -396,7 +428,15 @@ int pci_pwrctrl_create_devices(struct device *parent)
>  }
>  EXPORT_SYMBOL_GPL(pci_pwrctrl_create_devices);
>  
> -static void pci_pwrctrl_destroy_device(struct device_node *np)
> +/**
> + * pci_pwrctrl_destroy_device - Destroy pwrctrl devices
> + *
> + * @device_node: PCI port or device to operate on
> + *
> + * Recursively destroy pwrctrl devices for the devicetree hierarchy below
> + * the specified PCI port or device in a depth first manner.
> + */
> +void pci_pwrctrl_destroy_device(struct device_node *np)

[Severity: Low]
In these four kernel-doc comments in drivers/pci/pwrctrl/core.c, the parameter
is documented as @device_node instead of @np, pci_pwrctrl_create_device() is
missing @parent, and pci_pwrctrl_power_on_device() repeats "Before powering on
the device, make sure its driver is bound." twice. Could these be updated to
avoid W=1 kernel-doc warnings?

[ ... ]

> diff --git a/include/linux/pci-pwrctrl.h b/include/linux/pci-pwrctrl.h
> index 1192a2527521d..f27be6489b50f 100644
> --- a/include/linux/pci-pwrctrl.h
> +++ b/include/linux/pci-pwrctrl.h
> @@ -59,6 +59,10 @@ int pci_pwrctrl_create_devices(struct device *parent);
>  void pci_pwrctrl_destroy_devices(struct device *parent);
>  int pci_pwrctrl_power_on_devices(struct device *parent);
>  void pci_pwrctrl_power_off_devices(struct device *parent);
> +int pci_pwrctrl_create_device(struct device_node *np, struct device *parent);
> +void pci_pwrctrl_destroy_device(struct device_node *np);
> +int pci_pwrctrl_power_on_device(struct device_node *np);
> +void pci_pwrctrl_power_off_device(struct device_node *np);
>  #else
>  static inline int pci_pwrctrl_create_devices(struct device *parent) { return 0; }
>  static void pci_pwrctrl_destroy_devices(struct device *parent) { }

[Severity: Low]
Should include/linux/pci-pwrctrl.h also provide static inline stubs in the
#else (!CONFIG_PCI_PWRCTRL) branch for pci_pwrctrl_create_device(),
pci_pwrctrl_destroy_device(), pci_pwrctrl_power_on_device(), and
pci_pwrctrl_power_off_device(), along with a struct device_node forward
declaration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-apple-pcie-pwren-v2-0-216a4066965f@cyberchaos.dev?part=3

Re: [PATCH v2 2/3] PCI: apple: Integrate pwrctrl API

[Janne Grunau]

On Wed, Nov 04, 2026 at 12:40:30PM +0100, Yureka Lilian wrote:
> Integrate the PCI pwrctrl framework into the Apple PCIe host driver to
> provide standardized power management for PCI devices.
> 
> Notably, this allows enabling powering on the WiFi, SD card reader on
> various Macs by means of the pwrctrl framework before probing the ports.
> 
> Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
> ---
>  drivers/pci/controller/Kconfig      |  1 +
>  drivers/pci/controller/pcie-apple.c | 23 +++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index d246bbe37948..791892a6c0c6 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -46,6 +46,7 @@ config PCIE_APPLE
>  	depends on OF
>  	depends on PCI_MSI
>  	select PCI_HOST_COMMON
> +	select PCI_PWRCTRL_GENERIC
>  	select IRQ_MSI_LIB
>  	help
>  	  Say Y here if you want to enable PCIe controller support on Apple
> diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
> index c2cffc0659f4..2d32092e2122 100644
> --- a/drivers/pci/controller/pcie-apple.c
> +++ b/drivers/pci/controller/pcie-apple.c
> @@ -30,6 +30,7 @@
>  #include <linux/msi.h>
>  #include <linux/of_irq.h>
>  #include <linux/pci-ecam.h>
> +#include <linux/pci-pwrctrl.h>
>  
>  #include "pci-host-common.h"
>  
> @@ -825,6 +826,28 @@ static int apple_pcie_init(struct pci_config_window *cfg)
>  	if (WARN_ON(!pcie))
>  		return -ENOENT;
>  
> +	// Ensure all ports can be powered on before attempting initialization
> +	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> +		ret = pci_pwrctrl_create_device(of_port, pcie->dev);
> +		if (ret) {
> +			dev_err(pcie->dev, "Failed to create pwrctrl device: %pe\n", ERR_PTR(ret));

This should power down devices it already powered up and destroy the
created devices. As far as I can see -EINVAL is the only possible error
so the platform devices should be cleaned up

> +			return ret;
> +		}
> +
> +		ret = pci_pwrctrl_power_on_device(of_port);
> +		if (ret) {
> +			if (ret != -EPROBE_DEFER) {

-EPROBE_DEFER can be persistent, devices should be powered and
destroyed here as well.

> +				dev_err(pcie->dev,
> +					"Failed to power on device: %pe\n",
> +					ERR_PTR(ret));
> +				for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> +					pci_pwrctrl_destroy_device(of_port);

this misses to power down already powered up devices.

One way to avoid the need to power down devices after
pci_pwrctrl_create_device() failures would be to split the loop into
two.

The power handling is now done on a per port basis as asked in v1 but it
is more or less a re-implementation of pci_pwrctrl_create_devices(),
pci_pwrctrl_destroy_devices() and pci_pwrctrl_power_on_devices().
For pcie-apple those are essentially loops over the the available ports
with error handling.

One annoying side effect of this change. I a single device fails to
power-on we will break ethernet on desktop Macs although it does not
need power control. So maybe ignoring errors except -EPROBE_DEFER would
be the better implementation. -EPROBE_DEFER itself is still a problem as
the power control for some ports are a optional dependency for other
ports.

Janne

Re: [PATCH v2 3/3] arm64: dts: apple: t600x: Add PCIe pwren gpios

[Janne Grunau]

On Wed, Nov 04, 2026 at 12:40:31PM +0100, Yureka Lilian wrote:
> For the MacBook Pro and Mac Studio devices using the M1 Pro/Max/Ultra
> SoCs, add the appropriate fixed regulators for enabling the peripherals
> connected to the PCIe ports (WiFi, SD card reader and USB xHCI), which
> are controlled by GPIOs connected to the SMC.
> Set the regulators as the power-supply of the respective PCIe port, so
> that the endpoints are powered on before probing the ports.
> 
> Add the 'pciclass,0604' compatible string to the PCIe bridge nodes in
> the SoC to allow the pci-pwrctrl-generic driver to bind to them.
> 
> Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
> ---
>  arch/arm64/boot/dts/apple/t600x-die0.dtsi      |  4 ++++
>  arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi | 16 +++++++++++++++
>  arch/arm64/boot/dts/apple/t600x-j375.dtsi      | 27 ++++++++++++++++++++++++++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/apple/t600x-die0.dtsi b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
> index 122a566e75d1..635635c4bfbd 100644
> --- a/arch/arm64/boot/dts/apple/t600x-die0.dtsi
> +++ b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
> @@ -460,6 +460,7 @@ pcie0: pcie@590000000 {
>  		pinctrl-names = "default";
>  
>  		port00: pci@0,0 {
> +			compatible = "pciclass,0604";
>  			device_type = "pci";

`dt-check-style --mode strict` wants "device_type" before "compatible"

> diff --git a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
> index caded1636f3d..4f9dccf839ad 100644
> --- a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
> +++ b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
> @@ -55,6 +55,20 @@ led-0 {
>  			default-state = "keep";
>  		};
>  	};
> +
> +	vreg_pcie0_port0: regulator-pcie0-port0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwren-pcie0-port0";
> +		gpio = <&smc_gpio 13 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	vreg_pcie0_port1: regulator-pcie0-port1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwren-pcie0-port1";
> +		gpio = <&smc_gpio 26 GPIO_ACTIVE_HIGH>;

Please do the t602x changes in the same commit. This file is included by
t602x-j414-j416.dtsi and the M2 Pro/Max devices use different gpio pins.
If they aren't overriden in the same commit this will toggled a random
GPIO pin on M2 Pro/Max Macbook Pros.

> diff --git a/arch/arm64/boot/dts/apple/t600x-j375.dtsi b/arch/arm64/boot/dts/apple/t600x-j375.dtsi
> index 8a1494949e4c..412b2fe109be 100644
> --- a/arch/arm64/boot/dts/apple/t600x-j375.dtsi
> +++ b/arch/arm64/boot/dts/apple/t600x-j375.dtsi
> @@ -42,6 +42,30 @@ memory@10000000000 {
>  		device_type = "memory";
>  		reg = <0x100 0 0x2 0>; /* To be filled by loader */
>  	};
> +
> +	/* WLAN/BT */
> +	vreg_pcie0_port0: regulator-pcie0-port0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwren-pcie0-port0";
> +		gpio = <&smc_gpio 13 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	/* SD card reader */

this is included indirectly on t6020-j474.dts which doesn't have a card
reader.

> +	vreg_pcie0_port1: regulator-pcie0-port1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwren-pcie0-port1";
> +		gpio = <&smc_gpio 26 GPIO_ACTIVE_HIGH>;

Same comment about the GPIO pins as for the Macbook Pros.

I'm not sure if the cross-SoC generation device .dtsi files were a good
idea even though the devices are very similar. I probably wouldn't
repeat that but I don't think it's worth to untangle at this point.

Janne