* [PATCH 0/2] PCI: apple: Integrate pwrctrl API
@ 2026-07-06 22:38 Yureka Lilian
2026-07-06 22:38 ` [PATCH 1/2] " Yureka Lilian
2026-07-06 22:38 ` [PATCH 2/2] arm64: dts: apple: t600x: Add PCIe pwren gpios Yureka Lilian
0 siblings, 2 replies; 8+ messages in thread
From: Yureka Lilian @ 2026-07-06 22:38 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Marc Zyngier,
Sven Peter, Janne Grunau, Neal Gompa, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-pci, linux-kernel, asahi, linux-arm-kernel, devicetree,
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>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] PCI: apple: Integrate pwrctrl API 2026-07-06 22:38 [PATCH 0/2] PCI: apple: Integrate pwrctrl API Yureka Lilian @ 2026-07-06 22:38 ` Yureka Lilian 2026-07-06 22:59 ` sashiko-bot 2026-07-07 9:25 ` Marc Zyngier 2026-07-06 22:38 ` [PATCH 2/2] arm64: dts: apple: t600x: Add PCIe pwren gpios Yureka Lilian 1 sibling, 2 replies; 8+ messages in thread From: Yureka Lilian @ 2026-07-06 22:38 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Marc Zyngier, Sven Peter, Janne Grunau, Neal Gompa, Krzysztof Kozlowski, Conor Dooley Cc: linux-pci, linux-kernel, asahi, linux-arm-kernel, devicetree, 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PCI: apple: Integrate pwrctrl API 2026-07-06 22:38 ` [PATCH 1/2] " Yureka Lilian @ 2026-07-06 22:59 ` sashiko-bot 2026-07-07 9:25 ` Marc Zyngier 1 sibling, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-07-06 22:59 UTC (permalink / raw) To: Yureka Lilian; +Cc: robh, linux-pci, devicetree, conor+dt 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PCI: apple: Integrate pwrctrl API 2026-07-06 22:38 ` [PATCH 1/2] " Yureka Lilian 2026-07-06 22:59 ` sashiko-bot @ 2026-07-07 9:25 ` Marc Zyngier 2026-07-07 12:04 ` Yureka Lilian 1 sibling, 1 reply; 8+ messages in thread From: Marc Zyngier @ 2026-07-07 9:25 UTC (permalink / raw) To: Yureka Lilian Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Sven Peter, Janne Grunau, Neal Gompa, Krzysztof Kozlowski, Conor Dooley, linux-pci, linux-kernel, asahi, linux-arm-kernel, devicetree 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PCI: apple: Integrate pwrctrl API 2026-07-07 9:25 ` Marc Zyngier @ 2026-07-07 12:04 ` Yureka Lilian 2026-07-07 13:58 ` Marc Zyngier 0 siblings, 1 reply; 8+ messages in thread From: Yureka Lilian @ 2026-07-07 12:04 UTC (permalink / raw) To: Marc Zyngier, Yureka Lilian Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Sven Peter, Janne Grunau, Neal Gompa, Krzysztof Kozlowski, Conor Dooley, linux-pci, linux-kernel, asahi, linux-arm-kernel, devicetree 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. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PCI: apple: Integrate pwrctrl API 2026-07-07 12:04 ` Yureka Lilian @ 2026-07-07 13:58 ` Marc Zyngier 0 siblings, 0 replies; 8+ messages in thread From: Marc Zyngier @ 2026-07-07 13:58 UTC (permalink / raw) To: Yureka Lilian Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Sven Peter, Janne Grunau, Neal Gompa, Krzysztof Kozlowski, Conor Dooley, linux-pci, linux-kernel, asahi, linux-arm-kernel, devicetree 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] arm64: dts: apple: t600x: Add PCIe pwren gpios 2026-07-06 22:38 [PATCH 0/2] PCI: apple: Integrate pwrctrl API Yureka Lilian 2026-07-06 22:38 ` [PATCH 1/2] " Yureka Lilian @ 2026-07-06 22:38 ` Yureka Lilian 2026-07-06 22:58 ` sashiko-bot 1 sibling, 1 reply; 8+ messages in thread From: Yureka Lilian @ 2026-07-06 22:38 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Marc Zyngier, Sven Peter, Janne Grunau, Neal Gompa, Krzysztof Kozlowski, Conor Dooley Cc: linux-pci, linux-kernel, asahi, linux-arm-kernel, devicetree, 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] arm64: dts: apple: t600x: Add PCIe pwren gpios 2026-07-06 22:38 ` [PATCH 2/2] arm64: dts: apple: t600x: Add PCIe pwren gpios Yureka Lilian @ 2026-07-06 22:58 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-07-06 22:58 UTC (permalink / raw) To: Yureka Lilian; +Cc: devicetree, conor+dt, linux-pci, robh 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-07 13:58 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-06 22:38 [PATCH 0/2] PCI: apple: Integrate pwrctrl API Yureka Lilian 2026-07-06 22:38 ` [PATCH 1/2] " Yureka Lilian 2026-07-06 22:59 ` sashiko-bot 2026-07-07 9:25 ` Marc Zyngier 2026-07-07 12:04 ` Yureka Lilian 2026-07-07 13:58 ` Marc Zyngier 2026-07-06 22:38 ` [PATCH 2/2] arm64: dts: apple: t600x: Add PCIe pwren gpios Yureka Lilian 2026-07-06 22:58 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox