[PATCH V5 0/4] Add PCIe M.2 Key E connector support for NXP i.MX boards

[PATCH V5 0/4] Add PCIe M.2 Key E connector support for NXP i.MX boards

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

This series adds support for NXP Wi-Fi/BT combo chips (88W9098, AW693)
inserted into PCIe M.2 Key E connectors on i.MX95-19x19-EVK boards.

For M.2 cards that rely on PCIe L2 link state and wake-up mechanisms, the
card must remain powered during suspend. Patch #2 uses the existing
dw_pcie_rp::skip_pwrctrl_off flag to skip power-off during suspend and skip
power-on during the init path.

Also the btnxpuart driver is extended to obtain a pwrseq descriptor via the
OF graph on the UART controller device in patch #3.

---
Changes in V5:
1. Improve the patch #1-3 commit message.
2. Remove the skip_pwrctrl_off flag check in imx_pcie_shutdown() in
   patch #2.
3. Add an explicit pwrseq_power_off() call in probe error path and
   nxp_serdev_remove() in patch #3.
4. Remove the imx8mq/imx8qm/imx8qxp/imx8dxl old platforms dts changes to
   avoid pcie functional break in some cases. For example, Uboot without
   M.2 driver support but use the new kernel dts file directly. 

Changes in V4:
1. Add a seperate patch to move pci_pwrctrl_create_devices() to
   imx_pcie_probe() as suggested by Frank.
2. Rebase the dts patches based on [2] patchset.
3. Drop pwrseq-pcie-m2 ID patch as it got applied.

Changes in V3:
1. Move pci_pwrctrl_create_devices() to imx_pcie_probe() in parch #1,
   similar to other regulator_get calls as suggested by Frank.
2. Collected the tags.

Changes in V2:
1. Rebased on top of 7.1.0.
2. Removed the power_off error handling in patch #3 because pwrseq_put()
   would call pwrseq_power_off() to automatically clean.
3. Collected the tags.
---

Sherry Sun (4):
  PCI: imx6: Move pci_pwrctrl_create_devices() to imx_pcie_probe()
  PCI: imx6: Add skip_pwrctrl_off flag support
  Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
  arm64: dts: imx95-19x19-evk: Describe the PCIe M.2 Key E connector

 .../boot/dts/freescale/imx95-19x19-evk.dts    | 55 ++++++++++++++-----
 drivers/bluetooth/btnxpuart.c                 | 34 ++++++++++--
 drivers/pci/controller/dwc/pci-imx6.c         | 38 +++++++------
 3 files changed, 92 insertions(+), 35 deletions(-)


base-commit: b9810cd75b9fb56a3425d391cba3f608502bd474
-- 
2.50.1

[PATCH V5 1/4] PCI: imx6: Move pci_pwrctrl_create_devices() to imx_pcie_probe()

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

Previously, pci_pwrctrl_create_devices() was placed in
imx_pcie_host_init(), which is the .init callback of dw_pcie_host_ops.
This callback is invoked not only during probe, but also during resume.

This caused pci_pwrctrl_create_devices() to be called multiple
times across suspend/resume cycles, which is unnecessary since the
pwrctrl devices only need to be created once.

Move pci_pwrctrl_create_devices() to imx_pcie_probe() so that it is only
called once during probe, similar to other regulator_get calls.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index f55a68f60eea..92f8e4a299e8 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1382,16 +1382,10 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 		}
 	}
 
-	ret = pci_pwrctrl_create_devices(dev);
-	if (ret) {
-		dev_err(dev, "failed to create pwrctrl devices\n");
-		goto err_reg_disable;
-	}
-
 	ret = pci_pwrctrl_power_on_devices(dev);
 	if (ret) {
 		dev_err(dev, "failed to power on pwrctrl devices\n");
-		goto err_pwrctrl_destroy;
+		goto err_reg_disable;
 	}
 
 	ret = imx_pcie_clk_enable(imx_pcie);
@@ -1461,9 +1455,6 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 	imx_pcie_clk_disable(imx_pcie);
 err_pwrctrl_power_off:
 	pci_pwrctrl_power_off_devices(dev);
-err_pwrctrl_destroy:
-	if (ret != -EPROBE_DEFER)
-		pci_pwrctrl_destroy_devices(dev);
 err_reg_disable:
 	if (imx_pcie->vpcie)
 		regulator_disable(imx_pcie->vpcie);
@@ -1954,11 +1945,15 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = pci_pwrctrl_create_devices(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
+
 	pci->use_parent_dt_ranges = true;
 	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
 		ret = imx_add_pcie_ep(imx_pcie, pdev);
 		if (ret < 0)
-			return ret;
+			goto err_pwrctrl_destroy;
 
 		/*
 		 * FIXME: Only single Device (EPF) is supported due to the
@@ -1973,7 +1968,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
 		pci->pp.use_atu_msg = true;
 		ret = dw_pcie_host_init(&pci->pp);
 		if (ret < 0)
-			return ret;
+			goto err_pwrctrl_destroy;
 
 		if (pci_msi_enabled()) {
 			u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
@@ -1985,6 +1980,11 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	}
 
 	return 0;
+
+err_pwrctrl_destroy:
+	if (ret != -EPROBE_DEFER)
+		pci_pwrctrl_destroy_devices(dev);
+	return ret;
 }
 
 static void imx_pcie_shutdown(struct platform_device *pdev)
-- 
2.50.1

[PATCH V5 2/4] PCI: imx6: Add skip_pwrctrl_off flag support

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

Use dw_pcie_rp::skip_pwrctrl_off to avoid powering off devices during
suspend to preserve wakeup capability of the devices and also not to
power on the devices in the init path.

This allows controller power-off to be skipped when some devices (e.g.
M.2 Key E cards without auxiliary power) need to support PCIe L2 link
state and wake-up mechanisms.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 92f8e4a299e8..afcf3b6bf3cd 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1382,10 +1382,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 		}
 	}
 
-	ret = pci_pwrctrl_power_on_devices(dev);
-	if (ret) {
-		dev_err(dev, "failed to power on pwrctrl devices\n");
-		goto err_reg_disable;
+	if (!pp->skip_pwrctrl_off) {
+		ret = pci_pwrctrl_power_on_devices(dev);
+		if (ret) {
+			dev_err(dev, "failed to power on pwrctrl devices\n");
+			goto err_reg_disable;
+		}
 	}
 
 	ret = imx_pcie_clk_enable(imx_pcie);
@@ -1454,7 +1456,8 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 err_clk_disable:
 	imx_pcie_clk_disable(imx_pcie);
 err_pwrctrl_power_off:
-	pci_pwrctrl_power_off_devices(dev);
+	if (!pp->skip_pwrctrl_off)
+		pci_pwrctrl_power_off_devices(dev);
 err_reg_disable:
 	if (imx_pcie->vpcie)
 		regulator_disable(imx_pcie->vpcie);
@@ -1473,7 +1476,8 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
 	}
 	imx_pcie_clk_disable(imx_pcie);
 
-	pci_pwrctrl_power_off_devices(pci->dev);
+	if (!pci->pp.skip_pwrctrl_off)
+		pci_pwrctrl_power_off_devices(pci->dev);
 	if (imx_pcie->vpcie)
 		regulator_disable(imx_pcie->vpcie);
 }
-- 
2.50.1

[PATCH V5 3/4] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

Power supply to the M.2 Bluetooth device attached to the host using M.2
connector is controlled using the 'uart' pwrseq device. So add support
for getting the pwrseq device if the OF graph link is present.

Once obtained, pwrseq_power_on() is called to power up the M.2 Bluetooth
card. The power sequencer descriptor is obtained via devm_pwrseq_get()
with the UART controller device (serdev->ctrl->dev), since the OF graph
link is defined on the UART controller node.

The devm cleanup of the pwrseq descriptor is tied to the UART controller
device, not the serdev child device, so it does not fire when the serdev
probe fails and nxp_serdev_remove(), an explicit pwrseq_power_off() call
is added in probe error path and nxp_serdev_remove() to ensure the power
reference stay balanced.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Hi Frank and Bartosz, this V5 patch adds an explicit pwrseq_power_off()
call in both probe error path and nxp_serdev_remove() to properly handle
power references. I'm keeping your Reviewed-by tag. I can remove it if you
think it's inappropriate.
---
 drivers/bluetooth/btnxpuart.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 0bb300eef157..68f84c324113 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -9,6 +9,8 @@
 
 #include <linux/serdev.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/pwrseq/consumer.h>
 #include <linux/skbuff.h>
 #include <linux/unaligned.h>
 #include <linux/firmware.h>
@@ -211,6 +213,7 @@ struct btnxpuart_dev {
 
 	struct ps_data psdata;
 	struct btnxpuart_data *nxp_data;
+	struct pwrseq_desc *pwrseq;
 	struct reset_control *pdn;
 	struct hci_uart hu;
 };
@@ -1872,11 +1875,26 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
 		return err;
 	}
 
+	if (of_graph_is_present(dev_of_node(&serdev->ctrl->dev))) {
+		struct pwrseq_desc *pwrseq;
+
+		pwrseq = devm_pwrseq_get(&serdev->ctrl->dev, "uart");
+		if (IS_ERR(pwrseq))
+			return dev_err_probe(&serdev->dev, PTR_ERR(pwrseq),
+					     "failed to get pwrseq\n");
+
+		nxpdev->pwrseq = pwrseq;
+		err = pwrseq_power_on(pwrseq);
+		if (err)
+			return err;
+	}
+
 	/* Initialize and register HCI device */
 	hdev = hci_alloc_dev();
 	if (!hdev) {
 		dev_err(&serdev->dev, "Can't allocate HCI device\n");
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto err_pwrseq_power_off;
 	}
 
 	reset_control_deassert(nxpdev->pdn);
@@ -1907,13 +1925,16 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
 	if (bacmp(&ba, BDADDR_ANY))
 		hci_set_quirk(hdev, HCI_QUIRK_USE_BDADDR_PROPERTY);
 
-	if (hci_register_dev(hdev) < 0) {
+	err = hci_register_dev(hdev);
+	if (err < 0) {
 		dev_err(&serdev->dev, "Can't register HCI device\n");
 		goto probe_fail;
 	}
 
-	if (ps_setup(hdev))
+	if (ps_setup(hdev)) {
+		err = -ENODEV;
 		goto probe_fail_unregister;
+	}
 
 	hci_devcd_register(hdev, nxp_coredump, nxp_coredump_hdr,
 			   nxp_coredump_notify);
@@ -1925,7 +1946,10 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
 probe_fail:
 	reset_control_assert(nxpdev->pdn);
 	hci_free_dev(hdev);
-	return -ENODEV;
+err_pwrseq_power_off:
+	if (nxpdev->pwrseq)
+		pwrseq_power_off(nxpdev->pwrseq);
+	return err;
 }
 
 static void nxp_serdev_remove(struct serdev_device *serdev)
@@ -1952,6 +1976,8 @@ static void nxp_serdev_remove(struct serdev_device *serdev)
 	ps_cleanup(nxpdev);
 	hci_unregister_dev(hdev);
 	reset_control_assert(nxpdev->pdn);
+	if (nxpdev->pwrseq)
+		pwrseq_power_off(nxpdev->pwrseq);
 	hci_free_dev(hdev);
 }
 
-- 
2.50.1

[PATCH V5 4/4] arm64: dts: imx95-19x19-evk: Describe the PCIe M.2 Key E connector

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

The i.MX95-19x19-EVK has the PCIe M.2 Mechanical Key E connector to
connect wireless connectivity cards over PCIe and UART interfaces. Hence,
describe the connector node and link it with the PCIe 0 Root Port and
LPUART5 nodes through graph port/endpoint.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 .../boot/dts/freescale/imx95-19x19-evk.dts    | 55 ++++++++++++++-----
 1 file changed, 41 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
index 340ab0253ec2..1ed1a8282edc 100644
--- a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
@@ -57,6 +57,37 @@ memory@80000000 {
 		reg = <0x0 0x80000000 0 0x80000000>;
 	};
 
+	m2-connector {
+		compatible = "pcie-m2-e-connector";
+		vpcie3v3-supply = <&reg_m2_pwr>;
+		w-disable1-gpios = <&i2c7_pcal6524 6 GPIO_ACTIVE_LOW>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+				m2_e_pcie_ep: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&pcie0_port0_ep>;
+				};
+			};
+
+			port@3 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <3>;
+				m2_e_uart_ep: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&lpuart5_ep>;
+				};
+			};
+		};
+	};
+
 	fan0: pwm-fan {
 		compatible = "pwm-fan";
 		#cooling-cells = <2>;
@@ -145,16 +176,6 @@ reg_m2_pwr: regulator-m2-pwr {
 		startup-delay-us = <5000>;
 	};
 
-	reg_pcie0: regulator-pcie {
-		compatible = "regulator-fixed";
-		regulator-name = "PCIE_WLAN_EN";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-		vin-supply = <&reg_m2_pwr>;
-		gpio = <&i2c7_pcal6524 6 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
-	};
-
 	reg_slot_pwr: regulator-slot-pwr {
 		compatible = "regulator-fixed";
 		regulator-name = "PCIe slot-power";
@@ -477,8 +498,10 @@ &lpuart5 {
 	pinctrl-0 = <&pinctrl_uart5>;
 	status = "okay";
 
-	bluetooth {
-		compatible = "nxp,88w8987-bt";
+	port {
+		lpuart5_ep: endpoint {
+			remote-endpoint = <&m2_e_uart_ep>;
+		};
 	};
 };
 
@@ -555,8 +578,12 @@ &pcie0_ep {
 
 &pcie0_port0 {
 	reset-gpios = <&i2c7_pcal6524 5 GPIO_ACTIVE_LOW>;
-	vpcie3v3-supply = <&reg_pcie0>;
-	vpcie3v3aux-supply = <&reg_pcie0>;
+
+	port {
+		pcie0_port0_ep: endpoint {
+			remote-endpoint = <&m2_e_pcie_ep>;
+		};
+	};
 };
 
 &pcie1 {
-- 
2.50.1

Re: [PATCH V5 1/4] PCI: imx6: Move pci_pwrctrl_create_devices() to imx_pcie_probe()

[sashiko-bot]

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

Pre-existing issues:
- [High] In Endpoint (EP) mode, `imx_add_pcie_ep()` ignores the return value of `imx_pcie_host_init()` and fails to clean up on error, leading to a kernel panic.
- [High] Missing PM domain detach and device link deletion in `imx_pcie_probe()` error paths causes permanent probe failure on `-EPROBE_DEFER`.
--

--- Patch [1]: [PATCH V5 1/4] PCI: imx6: Move pci_pwrctrl_create_devices() to imx_pcie_probe() ---
commit c4ce7635410a21f29d4b7bcc43f48a82e9c31e5b
Author: Sherry Sun <sherry.sun@nxp.com>

PCI: imx6: Move pci_pwrctrl_create_devices() to imx_pcie_probe()

This commit moves pci_pwrctrl_create_devices() from imx_pcie_host_init()
to imx_pcie_probe(). This ensures the power control devices are created
only once during probe, avoiding duplicate creation across suspend and
resume cycles.

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index f55a68f60eea9..92f8e4a299e88 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c

[ ... ]

> @@ -1954,11 +1945,15 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = pci_pwrctrl_create_devices(dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
> +
>  	pci->use_parent_dt_ranges = true;
>  	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
>  		ret = imx_add_pcie_ep(imx_pcie, pdev);

[Severity: High]
This is a pre-existing issue, but does imx_add_pcie_ep() need to check the
return value of imx_pcie_host_init()?

If imx_pcie_host_init() fails (for example, returning -EPROBE_DEFER) and
powers down the hardware, imx_add_pcie_ep() completely ignores this and
continues execution:

imx_add_pcie_ep()
    imx_pcie_host_init(pp); /* Return value ignored */
    ep = &pci->ep;
    ...
    ret = dw_pcie_ep_init(ep); /* Writes to hardware registers */

Writing to unclocked or unpowered peripheral registers on i.MX causes an
asynchronous external abort (System Error / Data Abort). Could this result
in an immediate kernel panic during boot if imx_pcie_host_init() fails?

>  		if (ret < 0)
> -			return ret;
> +			goto err_pwrctrl_destroy;

[ ... ]

> @@ -1985,6 +1980,11 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	}
>  
>  	return 0;
> +
> +err_pwrctrl_destroy:
> +	if (ret != -EPROBE_DEFER)
> +		pci_pwrctrl_destroy_devices(dev);
> +	return ret;

[Severity: High]
This is a pre-existing issue, but does this error path also need to detach
the PM domains and delete device links?

Earlier in imx_pcie_probe(), imx_pcie_attach_pd() dynamically allocates
virtual PM domain devices and creates stateless device links. If the probe
fails and jumps here (for instance, if pci_pwrctrl_create_devices() or
imx_add_pcie_ep() returns -EPROBE_DEFER), these virtual devices and device
links are not cleaned up.

On the next probe attempt, dev_pm_domain_attach_by_name() attempts to
register a new virtual device with the exact same name, which causes a
sysfs/kobject naming collision.

Could this missing cleanup cause device_register() to fail with -EEXIST,
permanently preventing the PCIe controller from probing if it defers once
during boot?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709101555.3034853-1-sherry.sun@oss.nxp.com?part=1

Re: [PATCH V5 2/4] PCI: imx6: Add skip_pwrctrl_off flag support

[sashiko-bot]

> From: Sherry Sun <sherry.sun@nxp.com>
> 
> Use dw_pcie_rp::skip_pwrctrl_off to avoid powering off devices during
> suspend to preserve wakeup capability of the devices and also not to
> power on the devices in the init path.
> 
> This allows controller power-off to be skipped when some devices (e.g.
> M.2 Key E cards without auxiliary power) need to support PCIe L2 link
> state and wake-up mechanisms.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709101555.3034853-1-sherry.sun@oss.nxp.com?part=2

Re: [PATCH V5 3/4] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Calling `devm_pwrseq_get(&serdev->ctrl->dev)` in the child device's probe function binds the resource cleanup to the parent UART controller device, causing a permanent resource and memory leak on probe failures or driver unbind.
--

--- Patch [3]: [PATCH V5 3/4] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq ---
commit e33ed957970d3c81ac733b1991f81b42afce6a94
Author: Sherry Sun <sherry.sun@nxp.com>

Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
    
Power supply to the M.2 Bluetooth device attached to the host using M.2
connector is controlled using the 'uart' pwrseq device. So add support
for getting the pwrseq device if the OF graph link is present.

> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> index 0bb300eef157c..68f84c324113d 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c

[ ... ]

> @@ -1872,11 +1875,26 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
>  		return err;
>  	}
>  
> +	if (of_graph_is_present(dev_of_node(&serdev->ctrl->dev))) {
> +		struct pwrseq_desc *pwrseq;
> +
> +		pwrseq = devm_pwrseq_get(&serdev->ctrl->dev, "uart");

[Severity: High]
The commit message notes that the devm cleanup is tied to the UART controller
device and adds explicit pwrseq_power_off() calls to compensate, but does
this approach leak memory and pwrseq references during bind/unbind cycles?

Because devm_pwrseq_get() is called with the parent device
(&serdev->ctrl->dev), each time nxp_serdev_probe() is executed, a new devres
entry is allocated and attached to the parent's lifecycle.

If the probe fails later, or if the driver is unbound and re-bound, the
child device's devres list is cleaned up, but the parent's devres list
retains the accumulated allocations.

While pwrseq_power_off() balances the power state in the error and remove
paths, the pwrseq descriptor itself is never released. Could this prevent
the power sequencer provider module from ever being unloaded due to the
leaked references?

> +		if (IS_ERR(pwrseq))
> +			return dev_err_probe(&serdev->dev, PTR_ERR(pwrseq),
> +					     "failed to get pwrseq\n");
> +
> +		nxpdev->pwrseq = pwrseq;
> +		err = pwrseq_power_on(pwrseq);
> +		if (err)
> +			return err;
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709101555.3034853-1-sherry.sun@oss.nxp.com?part=3

Re: [PATCH V5 4/4] arm64: dts: imx95-19x19-evk: Describe the PCIe M.2 Key E connector

[sashiko-bot]

> From: Sherry Sun <sherry.sun@nxp.com>
> 
> The i.MX95-19x19-EVK has the PCIe M.2 Mechanical Key E connector to
> connect wireless connectivity cards over PCIe and UART interfaces. Hence,
> describe the connector node and link it with the PCIe 0 Root Port and
> LPUART5 nodes through graph port/endpoint.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709101555.3034853-1-sherry.sun@oss.nxp.com?part=4

Re: [PATCH V5 1/4] PCI: imx6: Move pci_pwrctrl_create_devices() to imx_pcie_probe()

[Frank Li]

On Thu, Jul 09, 2026 at 06:15:52PM +0800, Sherry Sun (OSS) wrote:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> Previously, pci_pwrctrl_create_devices() was placed in
> imx_pcie_host_init(), which is the .init callback of dw_pcie_host_ops.
> This callback is invoked not only during probe, but also during resume.
>
> This caused pci_pwrctrl_create_devices() to be called multiple
> times across suspend/resume cycles, which is unnecessary since the
> pwrctrl devices only need to be created once.
>
> Move pci_pwrctrl_create_devices() to imx_pcie_probe() so that it is only
> called once during probe, similar to other regulator_get calls.
>
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/pci/controller/dwc/pci-imx6.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index f55a68f60eea..92f8e4a299e8 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1382,16 +1382,10 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  		}
>  	}
>
> -	ret = pci_pwrctrl_create_devices(dev);
> -	if (ret) {
> -		dev_err(dev, "failed to create pwrctrl devices\n");
> -		goto err_reg_disable;
> -	}
> -
>  	ret = pci_pwrctrl_power_on_devices(dev);
>  	if (ret) {
>  		dev_err(dev, "failed to power on pwrctrl devices\n");
> -		goto err_pwrctrl_destroy;
> +		goto err_reg_disable;
>  	}
>
>  	ret = imx_pcie_clk_enable(imx_pcie);
> @@ -1461,9 +1455,6 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  	imx_pcie_clk_disable(imx_pcie);
>  err_pwrctrl_power_off:
>  	pci_pwrctrl_power_off_devices(dev);
> -err_pwrctrl_destroy:
> -	if (ret != -EPROBE_DEFER)
> -		pci_pwrctrl_destroy_devices(dev);
>  err_reg_disable:
>  	if (imx_pcie->vpcie)
>  		regulator_disable(imx_pcie->vpcie);
> @@ -1954,11 +1945,15 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> +	ret = pci_pwrctrl_create_devices(dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n");
> +
>  	pci->use_parent_dt_ranges = true;
>  	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
>  		ret = imx_add_pcie_ep(imx_pcie, pdev);
>  		if (ret < 0)
> -			return ret;
> +			goto err_pwrctrl_destroy;
>
>  		/*
>  		 * FIXME: Only single Device (EPF) is supported due to the
> @@ -1973,7 +1968,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  		pci->pp.use_atu_msg = true;
>  		ret = dw_pcie_host_init(&pci->pp);
>  		if (ret < 0)
> -			return ret;
> +			goto err_pwrctrl_destroy;
>
>  		if (pci_msi_enabled()) {
>  			u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> @@ -1985,6 +1980,11 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	}
>
>  	return 0;
> +
> +err_pwrctrl_destroy:
> +	if (ret != -EPROBE_DEFER)
> +		pci_pwrctrl_destroy_devices(dev);
> +	return ret;
>  }
>
>  static void imx_pcie_shutdown(struct platform_device *pdev)
> --
> 2.50.1
>
>

Re: [PATCH V5 2/4] PCI: imx6: Add skip_pwrctrl_off flag support

[Frank Li]

On Thu, Jul 09, 2026 at 06:15:53PM +0800, Sherry Sun (OSS) wrote:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> Use dw_pcie_rp::skip_pwrctrl_off to avoid powering off devices during
> suspend to preserve wakeup capability of the devices and also not to
> power on the devices in the init path.
>
> This allows controller power-off to be skipped when some devices (e.g.
> M.2 Key E cards without auxiliary power) need to support PCIe L2 link
> state and wake-up mechanisms.
>
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 92f8e4a299e8..afcf3b6bf3cd 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1382,10 +1382,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  		}
>  	}
>
> -	ret = pci_pwrctrl_power_on_devices(dev);
> -	if (ret) {
> -		dev_err(dev, "failed to power on pwrctrl devices\n");
> -		goto err_reg_disable;
> +	if (!pp->skip_pwrctrl_off) {

if pci_pwrctrl_power_on_devices is true, where call pci_pwrctrl_power_on_devices()

> +		ret = pci_pwrctrl_power_on_devices(dev);
> +		if (ret) {
> +			dev_err(dev, "failed to power on pwrctrl devices\n");
> +			goto err_reg_disable;
> +		}
>  	}
>
>  	ret = imx_pcie_clk_enable(imx_pcie);
> @@ -1454,7 +1456,8 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  err_clk_disable:
>  	imx_pcie_clk_disable(imx_pcie);
>  err_pwrctrl_power_off:
> -	pci_pwrctrl_power_off_devices(dev);
> +	if (!pp->skip_pwrctrl_off)
> +		pci_pwrctrl_power_off_devices(dev);
>  err_reg_disable:
>  	if (imx_pcie->vpcie)
>  		regulator_disable(imx_pcie->vpcie);
> @@ -1473,7 +1476,8 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
>  	}
>  	imx_pcie_clk_disable(imx_pcie);
>
> -	pci_pwrctrl_power_off_devices(pci->dev);
> +	if (!pci->pp.skip_pwrctrl_off)
> +		pci_pwrctrl_power_off_devices(pci->dev);

Not sure if there are counter in side power_(on|off) function, if not,
just skip power_off is enough.

Frank

>  	if (imx_pcie->vpcie)
>  		regulator_disable(imx_pcie->vpcie);
>  }
> --
> 2.50.1
>
>

RE: [PATCH V5 2/4] PCI: imx6: Add skip_pwrctrl_off flag support

[Sherry Sun]

> Subject: Re: [PATCH V5 2/4] PCI: imx6: Add skip_pwrctrl_off flag support
> 
> On Thu, Jul 09, 2026 at 06:15:53PM +0800, Sherry Sun (OSS) wrote:
> > From: Sherry Sun <sherry.sun@nxp.com>
> >
> > Use dw_pcie_rp::skip_pwrctrl_off to avoid powering off devices during
> > suspend to preserve wakeup capability of the devices and also not to
> > power on the devices in the init path.
> >
> > This allows controller power-off to be skipped when some devices (e.g.
> > M.2 Key E cards without auxiliary power) need to support PCIe L2 link
> > state and wake-up mechanisms.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 92f8e4a299e8..afcf3b6bf3cd 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1382,10 +1382,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> *pp)
> >  		}
> >  	}
> >
> > -	ret = pci_pwrctrl_power_on_devices(dev);
> > -	if (ret) {
> > -		dev_err(dev, "failed to power on pwrctrl devices\n");
> > -		goto err_reg_disable;
> > +	if (!pp->skip_pwrctrl_off) {
> 
> if pci_pwrctrl_power_on_devices is true, where call
> pci_pwrctrl_power_on_devices()

Hi Frank,
The skip_pwrctrl_off flag defaults to false during the pcie bus probing phase,
so the pci_pwrctrl_power_on_devices() is called at least once to ensure all
regulators are enabled.
The value of skip_pwrctrl_off is only changed in dw_pcie_suspend_noirq(),
and is refreshed by calling the pci_host_common_d3cold_possible() each
time a suspend occurs.
This is why we use this flag at runtime to avoid powering off devices during
suspend to preserve wakeup capability of the devices.

> 
> > +		ret = pci_pwrctrl_power_on_devices(dev);
> > +		if (ret) {
> > +			dev_err(dev, "failed to power on pwrctrl devices\n");
> > +			goto err_reg_disable;
> > +		}
> >  	}
> >
> >  	ret = imx_pcie_clk_enable(imx_pcie); @@ -1454,7 +1456,8 @@ static
> > int imx_pcie_host_init(struct dw_pcie_rp *pp)
> >  err_clk_disable:
> >  	imx_pcie_clk_disable(imx_pcie);
> >  err_pwrctrl_power_off:
> > -	pci_pwrctrl_power_off_devices(dev);
> > +	if (!pp->skip_pwrctrl_off)
> > +		pci_pwrctrl_power_off_devices(dev);
> >  err_reg_disable:
> >  	if (imx_pcie->vpcie)
> >  		regulator_disable(imx_pcie->vpcie);
> > @@ -1473,7 +1476,8 @@ static void imx_pcie_host_exit(struct dw_pcie_rp
> *pp)
> >  	}
> >  	imx_pcie_clk_disable(imx_pcie);
> >
> > -	pci_pwrctrl_power_off_devices(pci->dev);
> > +	if (!pci->pp.skip_pwrctrl_off)
> > +		pci_pwrctrl_power_off_devices(pci->dev);
> 
> Not sure if there are counter in side power_(on|off) function, if not, just skip
> power_off is enough.

It depends on the pwrctrl drivers implementation, for the generic pwrctrl, it will
call regulator_bulk_enable/disable() to handle this, so has enable/use counter.

As mentioned above, the value of skip_pwrctrl_off is refreshed in
dw_pcie_suspend_noirq() for each suspend/resume, so we need this runtime
check to avoid powering off some specific devices during suspend.

Best Regards
Sherry

RE: [PATCH V5 3/4] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq

[Sherry Sun]

> Subject: Re: [PATCH V5 3/4] Bluetooth: btnxpuart: Add M.2 Bluetooth device
> support using pwrseq
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [High] Calling `devm_pwrseq_get(&serdev->ctrl->dev)` in the child device's
> probe function binds the resource cleanup to the parent UART controller
> device, causing a permanent resource and memory leak on probe failures or
> driver unbind.
> --
> 
> --- Patch [3]: [PATCH V5 3/4] Bluetooth: btnxpuart: Add M.2 Bluetooth device
> support using pwrseq --- commit
> e33ed957970d3c81ac733b1991f81b42afce6a94
> Author: Sherry Sun <sherry.sun@nxp.com>
> 
> Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
> 
> Power supply to the M.2 Bluetooth device attached to the host using M.2
> connector is controlled using the 'uart' pwrseq device. So add support for
> getting the pwrseq device if the OF graph link is present.
> 
> > diff --git a/drivers/bluetooth/btnxpuart.c
> > b/drivers/bluetooth/btnxpuart.c index 0bb300eef157c..68f84c324113d
> > 100644
> > --- a/drivers/bluetooth/btnxpuart.c
> > +++ b/drivers/bluetooth/btnxpuart.c
> 
> [ ... ]
> 
> > @@ -1872,11 +1875,26 @@ static int nxp_serdev_probe(struct
> serdev_device *serdev)
> >  		return err;
> >  	}
> >
> > +	if (of_graph_is_present(dev_of_node(&serdev->ctrl->dev))) {
> > +		struct pwrseq_desc *pwrseq;
> > +
> > +		pwrseq = devm_pwrseq_get(&serdev->ctrl->dev, "uart");
> 
> [Severity: High]
> The commit message notes that the devm cleanup is tied to the UART
> controller device and adds explicit pwrseq_power_off() calls to compensate,
> but does this approach leak memory and pwrseq references during
> bind/unbind cycles?
> 
> Because devm_pwrseq_get() is called with the parent device (&serdev->ctrl-
> >dev), each time nxp_serdev_probe() is executed, a new devres entry is
> allocated and attached to the parent's lifecycle.
> 
> If the probe fails later, or if the driver is unbound and re-bound, the child
> device's devres list is cleaned up, but the parent's devres list retains the
> accumulated allocations.
> 
> While pwrseq_power_off() balances the power state in the error and remove
> paths, the pwrseq descriptor itself is never released. Could this prevent the
> power sequencer provider module from ever being unloaded due to the
> leaked references?

Yes, this is true issue, will fix in V6.

Best Regards
Sherry

> 
> > +		if (IS_ERR(pwrseq))
> > +			return dev_err_probe(&serdev->dev,
> PTR_ERR(pwrseq),
> > +					     "failed to get pwrseq\n");
> > +
> > +		nxpdev->pwrseq = pwrseq;
> > +		err = pwrseq_power_on(pwrseq);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> 
> --
> Sashiko AI
> review * https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> Fsashiko.dev%2F%23%2Fpatchset%2F20260709101555.3034853-1-
> sherry.sun%40oss.nxp.com%3Fpart%3D3&data=05%7C02%7Csherry.sun%40
> nxp.com%7C48b334e953744676329608deddaaa9c6%7C686ea1d3bc2b4c6fa9
> 2cd99c5c301635%7C0%7C0%7C639191922250049941%7CUnknown%7CTWFp
> bGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4z
> MiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=G5jMhoI6
> dW0e5QpHqX%2FWNupCSiSPwAlkEa0Afo0tzpI%3D&reserved=0

Re: [PATCH V5 2/4] PCI: imx6: Add skip_pwrctrl_off flag support

[Frank Li]

On Fri, Jul 10, 2026 at 02:16:35AM +0000, Sherry Sun wrote:
> > Subject: Re: [PATCH V5 2/4] PCI: imx6: Add skip_pwrctrl_off flag support
> >
> > On Thu, Jul 09, 2026 at 06:15:53PM +0800, Sherry Sun (OSS) wrote:
> > > From: Sherry Sun <sherry.sun@nxp.com>
> > >
> > > Use dw_pcie_rp::skip_pwrctrl_off to avoid powering off devices during
> > > suspend to preserve wakeup capability of the devices and also not to
> > > power on the devices in the init path.
> > >
> > > This allows controller power-off to be skipped when some devices (e.g.
> > > M.2 Key E cards without auxiliary power) need to support PCIe L2 link
> > > state and wake-up mechanisms.
> > >
> > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 16 ++++++++++------
> > >  1 file changed, 10 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 92f8e4a299e8..afcf3b6bf3cd 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -1382,10 +1382,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> > *pp)
> > >  		}
> > >  	}
> > >
> > > -	ret = pci_pwrctrl_power_on_devices(dev);
> > > -	if (ret) {
> > > -		dev_err(dev, "failed to power on pwrctrl devices\n");
> > > -		goto err_reg_disable;
> > > +	if (!pp->skip_pwrctrl_off) {
> >
> > if pci_pwrctrl_power_on_devices is true, where call
> > pci_pwrctrl_power_on_devices()
>
> Hi Frank,
> The skip_pwrctrl_off flag defaults to false during the pcie bus probing phase,
> so the pci_pwrctrl_power_on_devices() is called at least once to ensure all
> regulators are enabled.
> The value of skip_pwrctrl_off is only changed in dw_pcie_suspend_noirq(),
> and is refreshed by calling the pci_host_common_d3cold_possible() each
> time a suspend occurs.
> This is why we use this flag at runtime to avoid powering off devices during
> suspend to preserve wakeup capability of the devices.

Okay, It would be better this part code can move into common dwc later.
this is not specific for imx.

Reviewed-by: Frank Li <Frank.Li@nxp.com>

Frank

>
> >
> > > +		ret = pci_pwrctrl_power_on_devices(dev);
> > > +		if (ret) {
> > > +			dev_err(dev, "failed to power on pwrctrl devices\n");
> > > +			goto err_reg_disable;
> > > +		}
> > >  	}
> > >
> > >  	ret = imx_pcie_clk_enable(imx_pcie); @@ -1454,7 +1456,8 @@ static
> > > int imx_pcie_host_init(struct dw_pcie_rp *pp)
> > >  err_clk_disable:
> > >  	imx_pcie_clk_disable(imx_pcie);
> > >  err_pwrctrl_power_off:
> > > -	pci_pwrctrl_power_off_devices(dev);
> > > +	if (!pp->skip_pwrctrl_off)
> > > +		pci_pwrctrl_power_off_devices(dev);
> > >  err_reg_disable:
> > >  	if (imx_pcie->vpcie)
> > >  		regulator_disable(imx_pcie->vpcie);
> > > @@ -1473,7 +1476,8 @@ static void imx_pcie_host_exit(struct dw_pcie_rp
> > *pp)
> > >  	}
> > >  	imx_pcie_clk_disable(imx_pcie);
> > >
> > > -	pci_pwrctrl_power_off_devices(pci->dev);
> > > +	if (!pci->pp.skip_pwrctrl_off)
> > > +		pci_pwrctrl_power_off_devices(pci->dev);
> >
> > Not sure if there are counter in side power_(on|off) function, if not, just skip
> > power_off is enough.
>
> It depends on the pwrctrl drivers implementation, for the generic pwrctrl, it will
> call regulator_bulk_enable/disable() to handle this, so has enable/use counter.
>
> As mentioned above, the value of skip_pwrctrl_off is refreshed in
> dw_pcie_suspend_noirq() for each suspend/resume, so we need this runtime
> check to avoid powering off some specific devices during suspend.
>
> Best Regards
> Sherry