[PATCH 0/2] PCI: apple: Integrate pwrctrl API

[PATCH 0/2] PCI: apple: Integrate pwrctrl API

[Yureka Lilian]

Hi all,

this is my attempt at using the pci-pwrctrl framework to power on the
PCIe-connected WiFi and SD card reader on Apple Silicon Macs.
Back in 2022, Hector Martin proposed a custom solution for this[1].
Since then, the pci-pwrctrl framework has evolved and
pci-pwrctrl-generic seems to be the way to do this nowadays.
Device tree changes for t600x are included as an example for how this
could be used.

Link[1]: https://lore.kernel.org/lkml/20220502093832.32778-4-marcan@marcan.st/
Link[2]: https://lwn.net/Articles/1052333/

Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
---
Yureka Lilian (2):
      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            | 16 +++++++++++++++
 5 files changed, 64 insertions(+)
---
base-commit: 8e9685d3c41c35dd1b37df70d854137abcb2fbac
change-id: 20260706-apple-pcie-pwren-0dd1ebdf1c4c

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

[PATCH 1/2] 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.

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

Link[1]: https://lore.kernel.org/lkml/20220502093832.32778-4-marcan@marcan.st/

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

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 2247709ef6d6..af64630d28fa 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..db038a9d4831 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,21 @@ static int apple_pcie_init(struct pci_config_window *cfg)
 	if (WARN_ON(!pcie))
 		return -ENOENT;
 
+	ret = pci_pwrctrl_create_devices(pcie->dev);
+	if (ret) {
+		dev_err(pcie->dev, "Failed to create pwrctrl devices: %pe\n", ret);
+		return ret;
+	}
+
+	ret = pci_pwrctrl_power_on_devices(pcie->dev);
+	if (ret) {
+		if (ret != -EPROBE_DEFER) {
+			dev_err(pcie->dev, "Failed to power on devices: %pe\n", ret);
+			pci_pwrctrl_destroy_devices(pcie->dev);
+		}
+		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.54.0

[PATCH 2/2] 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 f715b19efd16..53a633a3c652 100644
--- a/arch/arm64/boot/dts/apple/t600x-die0.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
@@ -449,6 +449,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>;
@@ -468,6 +469,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>;
@@ -487,6 +489,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>;
@@ -507,6 +510,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 fee84f809a9c..7954cbbe4514 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.54.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The device tree changes are non-functional because the Apple PCIe controller driver lacks the required `pci-pwrctrl` framework integration, leaving the endpoints unpowered.
- [High] The fixed regulator nodes for the PCIe ports omit the `startup-delay-us` property, which will cause a violation of the PCIe CEM specification T_PVPERL timing constraint when enabled.
--

--- Patch [2]: [PATCH 2/2] arm64: dts: apple: t600x: Add PCIe pwren gpios ---
commit b2b3991e65bd2ca1aa384e079e00556daa346284
Author: Yureka Lilian <yureka@cyberchaos.dev>

arm64: dts: apple: t600x: Add PCIe pwren gpios
    
This commit adds fixed regulators to enable PCIe peripherals (WiFi, SD card
reader, and USB xHCI) on Apple M1 Pro/Max/Ultra SoCs. It sets these regulators
as the power-supply for the respective PCIe ports and adds the pciclass,0604
compatible string to allow the pci-pwrctrl-generic driver to bind.

> diff --git a/arch/arm64/boot/dts/apple/t600x-die0.dtsi b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
> index f715b19efd167..53a633a3c6527 100644
> --- a/arch/arm64/boot/dts/apple/t600x-die0.dtsi
> +++ b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
[ ... ]
> @@ -449,6 +449,7 @@ pcie0: pcie@590000000 {
>  		pinctrl-names = "default";
>  
>  		port00: pci@0,0 {
> +			compatible = "pciclass,0604";

[Severity: High]
Will this generic power control driver actually be probed?

The pci-pwrctrl-generic driver requires the PCIe host controller to explicitly
call pci_pwrctrl_create_devices() to instantiate platform devices for these
child nodes.

Looking at drivers/pci/controller/pcie-apple.c, this API is never invoked. As
a result, the pci-pwrctrl device is never created, the power-supply properties
will be ignored, and the connected endpoints will remain unpowered.

>  			device_type = "pci";
>  			reg = <0x0 0x0 0x0 0x0 0x0>;
>  			reset-gpios = <&pinctrl_ap 4 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 fee84f809a9cc..7954cbbe45140 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;
> +	};

[Severity: High]
Does this regulator need a startup-delay-us property to meet PCIe timing?

When the generic power control framework eventually manages this regulator
(either once the pci-pwrctrl driver is supported or via direct host controller
control), it will enable it without implicit delays. 

Without startup-delay-us = <100000>, regulator_enable() returns immediately.
The apple_pcie_setup_port() function then asserts PERST#, waits only 100us,
and deasserts PERST#. 

Could this result in a T_PVPERL of ~100us instead of the spec-mandated 100ms,
causing the PCIe endpoints to fail initialization or act erratically? (This
also applies to the other fixed regulators introduced in this patch).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-apple-pcie-pwren-v1-0-5a281b182fe2@cyberchaos.dev?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Missing cleanup of PCI pwrctrl devices on error paths in apple_pcie_init().
- [Medium] Using the `%pe` format specifier with an integer error code instead of an error pointer.

Pre-existing issues:
- [Critical] Use-After-Free and resource leak of port IRQ handlers and domains on probe failure.
- [High] MSI IRQ domain is permanently leaked on probe deferral, heavily exacerbated by the new pwrctrl deferred probing.
--

--- Patch [1]: [PATCH 1/2] PCI: apple: Integrate pwrctrl API ---
commit 937db91081c31bded4a8e5eaa8859cec8bb3fc57
Author: Yureka Lilian <yureka@cyberchaos.dev>
Subject: PCI: apple: Integrate pwrctrl API

This patch integrates the PCI pwrctrl framework into the Apple PCIe host
driver to provide standardized power management. It allows powering on
devices like WiFi and SD card readers using the pci-pwrctrl-generic
driver before probing the ports.

Link: https://lore.kernel.org/lkml/20220502093832.32778-4-marcan@marcan.st/

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

[ ... ]

> @@ -825,6 +826,21 @@ static int apple_pcie_init(struct pci_config_window *cfg)
>  	if (WARN_ON(!pcie))
>  		return -ENOENT;
>  
> +	ret = pci_pwrctrl_create_devices(pcie->dev);
> +	if (ret) {
> +		dev_err(pcie->dev, "Failed to create pwrctrl devices: %pe\n", ret);

[Severity: Medium]
Are we using the %pe format specifier with an integer error code instead
of a pointer here?

Passing the 32-bit integer ret directly might cause undefined behavior on
64-bit architectures, or print a hashed pointer address instead of the
actual error string since IS_ERR() evaluates to false.

Should this be cast with ERR_PTR(ret)?

> +		return ret;
> +	}
> +
> +	ret = pci_pwrctrl_power_on_devices(pcie->dev);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER) {
> +			dev_err(pcie->dev, "Failed to power on devices: %pe\n", ret);

[Severity: Medium]
Is this %pe format specifier also receiving the integer ret directly
instead of a pointer?

> +			pci_pwrctrl_destroy_devices(pcie->dev);
> +		}
> +		return ret;

[Severity: High]
This is a pre-existing issue, but does returning -EPROBE_DEFER here
exacerbate a memory leak in apple_msi_init()?

Because pci_pwrctrl_power_on_devices() relies on of_platform_device_create()
spawning child devices that probe asynchronously, it seems this might
frequently return -EPROBE_DEFER.

Earlier in the probe path, apple_msi_init() creates an MSI domain via
msi_create_parent_irq_domain(), but lacks a devm_add_action() to clean
it up.

If we defer probe here, do we permanently leak the previous MSI domain and
pcie->bitmap allocations on each retry?

> +	}
> +
>  	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
>  		ret = apple_pcie_setup_port(pcie, of_port);
>  		if (ret) {
>  			dev_err(dev, "Port %pOF setup fail: %d\n", of_port, ret);
>  			return ret;

[Severity: High]
If apple_pcie_setup_port() fails and we return ret here, do we bypass
tearing down the successfully created pwrctrl devices?

If pci_pwrctrl_create_devices() and pci_pwrctrl_power_on_devices()
succeeded earlier, it seems this error path leaves the hardware powered on
indefinitely and leaks the child platform devices since they are not
devres-managed.

Should we call pci_pwrctrl_power_off_devices() and
pci_pwrctrl_destroy_devices() before returning?

[Severity: Critical]
This isn't a bug introduced by this patch, but could this error path lead
to a use-after-free and resource leak of port IRQ handlers?

If the loop successfully sets up the first port but fails on a subsequent
port, the probe fails and devres frees the struct apple_pcie_port for
the successful port.

However, looking at apple_pcie_port_register_irqs(), it registers the
handler using request_irq() without devm:

drivers/pci/controller/pcie-apple.c:apple_pcie_port_register_irqs() {
    ...
    ret = request_irq(irq, apple_pcie_port_irq, 0, port_irqs[i].name, port);
    ...
}

Similarly, apple_pcie_port_setup_irq() uses
irq_set_chained_handler_and_data() without a devm teardown.

Since the IRQs remain active, could an incoming interrupt trigger the
handlers and dereference the freed port structure?

>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-apple-pcie-pwren-v1-0-5a281b182fe2@cyberchaos.dev?part=1

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

[Marc Zyngier]

On Mon, 06 Jul 2026 23:38:27 +0100,
Yureka Lilian <yureka@cyberchaos.dev> 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.
> 
> Previously, a custom solution for powering on the WiFi and SD card
> reader was proposed[1], but we can now use the new pci-pwrctrl-generic
> driver for this purpose.
> 
> Link[1]: https://lore.kernel.org/lkml/20220502093832.32778-4-marcan@marcan.st/
>

nit: this paragraph and the accompanying link don't belong in the
commit message and should be moved below the --- mark or even better,
to the cover letter.

> Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
> ---
>  drivers/pci/controller/Kconfig      |  1 +
>  drivers/pci/controller/pcie-apple.c | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 2247709ef6d6..af64630d28fa 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..db038a9d4831 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,21 @@ static int apple_pcie_init(struct pci_config_window *cfg)
>  	if (WARN_ON(!pcie))
>  		return -ENOENT;
>  
> +	ret = pci_pwrctrl_create_devices(pcie->dev);
> +	if (ret) {
> +		dev_err(pcie->dev, "Failed to create pwrctrl devices: %pe\n", ret);
> +		return ret;
> +	}
> +
> +	ret = pci_pwrctrl_power_on_devices(pcie->dev);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER) {
> +			dev_err(pcie->dev, "Failed to power on devices: %pe\n", ret);
> +			pci_pwrctrl_destroy_devices(pcie->dev);
> +		}
> +		return ret;
> +	}
> +

Why is this done globally while the whole driver works on a per-port
basis, and that the proposed DT updates are also per port?

Thanks,

	M.

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

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

[Yureka Lilian]

On 7/7/26 11:25, Marc Zyngier wrote:
> On Mon, 06 Jul 2026 23:38:27 +0100,
> Yureka Lilian <yureka@cyberchaos.dev> 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.
>>
>> Previously, a custom solution for powering on the WiFi and SD card
>> reader was proposed[1], but we can now use the new pci-pwrctrl-generic
>> driver for this purpose.
>>
>> Link[1]: https://lore.kernel.org/lkml/20220502093832.32778-4-marcan@marcan.st/
>>
> nit: this paragraph and the accompanying link don't belong in the
> commit message and should be moved below the --- mark or even better,
> to the cover letter.
ack, will leave it out of the commit message of the individual commit in v2
>> Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
>> ---
>>   drivers/pci/controller/Kconfig      |  1 +
>>   drivers/pci/controller/pcie-apple.c | 16 ++++++++++++++++
>>   2 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
>> index 2247709ef6d6..af64630d28fa 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..db038a9d4831 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,21 @@ static int apple_pcie_init(struct pci_config_window *cfg)
>>   	if (WARN_ON(!pcie))
>>   		return -ENOENT;
>>   
>> +	ret = pci_pwrctrl_create_devices(pcie->dev);
>> +	if (ret) {
>> +		dev_err(pcie->dev, "Failed to create pwrctrl devices: %pe\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = pci_pwrctrl_power_on_devices(pcie->dev);
>> +	if (ret) {
>> +		if (ret != -EPROBE_DEFER) {
>> +			dev_err(pcie->dev, "Failed to power on devices: %pe\n", ret);
>> +			pci_pwrctrl_destroy_devices(pcie->dev);
>> +		}
>> +		return ret;
>> +	}
>> +
> Why is this done globally while the whole driver works on a per-port
> basis, and that the proposed DT updates are also per port?

pci_pwrctrl_power_on_devices takes a struct device as parameter, but
pcie-apple does not allocate device structs for the individual ports.
This could be changed of course. But since pci_pwrctrl_* operate on
the subnodes recursively, it works just fine this way.
Additionally, we would like to be sure all the endpoints can be powered
before we start initializing the individual ports. Otherwise we could
end up in a situation where some ports are initialized but others are
not when we realize some driver needed to power on the endpoints for
port n is not yet bound. There is no appropriate pci_pwrctrl API for
checking for the availability without changing the state, so this is
an additional reason to do it early.

>
> Thanks,
>
> 	M.
>

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

[Marc Zyngier]

On Tue, 07 Jul 2026 13:04:21 +0100,
Yureka Lilian <yureka@cyberchaos.dev> wrote:
> 
> On 7/7/26 11:25, Marc Zyngier wrote:
> > On Mon, 06 Jul 2026 23:38:27 +0100,
> > Yureka Lilian <yureka@cyberchaos.dev> 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.
> >> 
> >> Previously, a custom solution for powering on the WiFi and SD card
> >> reader was proposed[1], but we can now use the new pci-pwrctrl-generic
> >> driver for this purpose.
> >> 
> >> Link[1]: https://lore.kernel.org/lkml/20220502093832.32778-4-marcan@marcan.st/
> >> 
> > nit: this paragraph and the accompanying link don't belong in the
> > commit message and should be moved below the --- mark or even better,
> > to the cover letter.
> ack, will leave it out of the commit message of the individual commit in v2
> >> Signed-off-by: Yureka Lilian <yureka@cyberchaos.dev>
> >> ---
> >>   drivers/pci/controller/Kconfig      |  1 +
> >>   drivers/pci/controller/pcie-apple.c | 16 ++++++++++++++++
> >>   2 files changed, 17 insertions(+)
> >> 
> >> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> >> index 2247709ef6d6..af64630d28fa 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..db038a9d4831 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,21 @@ static int apple_pcie_init(struct
> >> pci_config_window *cfg)
> >>   	if (WARN_ON(!pcie))
> >>   		return -ENOENT;
> >>   +	ret = pci_pwrctrl_create_devices(pcie->dev);
> >> +	if (ret) {
> >> +		dev_err(pcie->dev, "Failed to create pwrctrl devices: %pe\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	ret = pci_pwrctrl_power_on_devices(pcie->dev);
> >> +	if (ret) {
> >> +		if (ret != -EPROBE_DEFER) {
> >> +			dev_err(pcie->dev, "Failed to power on devices: %pe\n", ret);
> >> +			pci_pwrctrl_destroy_devices(pcie->dev);
> >> +		}
> >> +		return ret;
> >> +	}
> >> +
> > Why is this done globally while the whole driver works on a per-port
> > basis, and that the proposed DT updates are also per port?
> 
> pci_pwrctrl_power_on_devices takes a struct device as parameter, but
> pcie-apple does not allocate device structs for the individual ports.
> This could be changed of course. But since pci_pwrctrl_* operate on
> the subnodes recursively, it works just fine this way.

Works fine is one thing. Being consistent with the way the rest of the
driver works is another. pci_pwrctrl_create_device() and
pci_pwrctrl_power_on_device() appear to do exactly what would be
required for a single port. They just needs to be exported made
global/exported.

> Additionally, we would like to be sure all the endpoints can be powered
> before we start initializing the individual ports. Otherwise we could
> end up in a situation where some ports are initialized but others are
> not when we realize some driver needed to power on the endpoints for
> port n is not yet bound.

How can a driver be bound to a device connected to a port if we
haven't been through the initial link-up dance, which is what
apple_pcie_setup_port() does? The power supplies are per-port for a
reason.

> There is no appropriate pci_pwrctrl API for checking for the
> availability without changing the state, so this is an additional
> reason to do it early.

Availability of what? To be clear, I'm not suggesting delaying
switching of the power. I'm merely suggesting managing it on a per
port basis.

Thanks,

	M.

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