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

[PATCH V6 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 V6:
1. Fix the power sequencer reference leak issue during BT driver unbound
   and re-bound in patch #3 issue that reported by sashiko.
2. Collected the Reviewed-by tags.

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: bee763d5f341b99cf472afeb508d4988f62a6ca1
-- 
2.50.1

[PATCH V6 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>
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

[PATCH V6 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>
Reviewed-by: Frank Li <Frank.Li@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 V6 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 pwrseq_get() with
the UART controller device (serdev->ctrl->dev), since the OF graph
link is defined on the UART controller node.

Also add the explicit pwrseq_put() call in all exit paths, pwrseq_put()
already calls pwrseq_power_off() internally, so no separate
pwrseq_power_off() call is needed.

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 V6 patch using pwrseq_put() in all exit paths to
fix the power sequencer reference leak issue during BT driver unload and
reload as reported by sashiko. 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..81a11ac05114 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 = 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)
+			goto err_pwrseq_put;
+	}
+
 	/* 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_put;
 	}
 
 	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_put:
+	if (nxpdev->pwrseq)
+		pwrseq_put(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_put(nxpdev->pwrseq);
 	hci_free_dev(hdev);
 }
 
-- 
2.50.1

[PATCH V6 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 V6 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/20260713023435.235765-1-sherry.sun@oss.nxp.com?part=4

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

[sashiko-bot]

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

New issues:
- [High] Brittle logic causing probe failure on valid hardware topologies.

Pre-existing issues:
- [High] Race condition exposing device before initializing synchronization primitives.
- [High] Use-after-free and locking a destroyed mutex during device teardown.
--

--- Patch [3]: [PATCH V6 3/4] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq ---
commit 885432311fa71ac754d16037ae3e761e8192452c
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. This commit adds
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
> --- 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 = pwrseq_get(&serdev->ctrl->dev, "uart");
> +		if (IS_ERR(pwrseq))
> +			return dev_err_probe(&serdev->dev, PTR_ERR(pwrseq),
> +					     "failed to get pwrseq\n");

[Severity: High]
If the parent UART node has an OF graph for other purposes, like audio
routing or multiplexing, will this unconditionally fail the driver probe?

When of_graph_is_present() returns true, pwrseq_get() might return an
error like -ENOENT if there is no power sequencer configured for the
uart target.

Could this logic be made less brittle to support valid hardware topologies
where the UART has an OF graph but no power sequencer?

> +
> +		nxpdev->pwrseq = pwrseq;
> +		err = pwrseq_power_on(pwrseq);
> +		if (err)
> +			goto err_pwrseq_put;
> +	}

[ ... ]

> @@ -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;
> +	}

[Severity: High]
This is a pre-existing issue, but does calling hci_register_dev() before
ps_setup() create a race condition during initialization?

When hci_register_dev() is called, it schedules the power_on work
asynchronously, which opens the UART port via btnxpuart_open().

Once the UART is open, incoming data triggers the receive path:

btnxpuart_receive_buf()
  ps_wakeup()

ps_wakeup() attempts to lock psdata->ps_lock and schedule psdata->work.

If ps_setup() has not yet completed its initialization of these structures,
could this result in operating on an uninitialized mutex or workqueue?

>  
>  	hci_devcd_register(hdev, nxp_coredump, nxp_coredump_hdr,

[ ... ]

> @@ -1952,6 +1976,8 @@ static void nxp_serdev_remove(struct serdev_device *serdev)
>  		}
>  	}
>  
>  	ps_cleanup(nxpdev);
>  	hci_unregister_dev(hdev);

[Severity: High]
This is a pre-existing issue, but does calling ps_cleanup() before
hci_unregister_dev() cause a use-after-free or a lock on a destroyed mutex?

ps_cleanup() destroys psdata->ps_lock and cancels psdata->work.

However, hci_unregister_dev() shuts down the device. During this shutdown
process, the UART can still receive data or send commands.

If data is received or sent, the paths btnxpuart_receive_buf() to
ps_wakeup() or nxp_enqueue() will attempt to acquire the destroyed
psdata->ps_lock.

Should hci_unregister_dev() be called before cleaning up the
synchronization primitives?

>  	reset_control_assert(nxpdev->pdn);
> +	if (nxpdev->pwrseq)
> +		pwrseq_put(nxpdev->pwrseq);
>  	hci_free_dev(hdev);
>  }

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Hardware initialization failure is ignored in `imx_add_pcie_ep()`, likely leading to a system crash.
--

--- Patch [2]: [PATCH V6 2/4] PCI: imx6: Add skip_pwrctrl_off flag support ---
commit 6bc8a94c274f464b53bad3ac0ac1a464c280f05e
Author: Sherry Sun <sherry.sun@nxp.com>

PCI: imx6: Add skip_pwrctrl_off flag support

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.

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 92f8e4a299e88..afcf3b6bf3cd7 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);

[Severity: High]
This is a pre-existing issue, but since this patch modifies
imx_pcie_host_init(), I noticed an issue with how its return value is handled.

Looking at imx_add_pcie_ep() which calls this function:

drivers/pci/controller/dwc/pci-imx6.c:imx_add_pcie_ep() {
	...
	imx_pcie_host_init(pp);
	ep = &pci->ep;
	ep->ops = &pcie_ep_ops;
	...
}

The return value of imx_pcie_host_init() is entirely ignored here. If hardware
initialization fails (e.g., clocks fail to enable or PHY fails to power on),
imx_pcie_host_init() returns a negative error code but leaves the hardware
unclocked or unpowered.

The code then proceeds to call dw_pcie_ep_init_registers(), which accesses
unclocked DBI memory-mapped registers (e.g., PCI_HEADER_TYPE).

Can this lead to a synchronous external abort and kernel panic when accessing
these unclocked peripheral registers?

[ ... ]

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The return value of `imx_pcie_host_init()` is ignored in `imx_add_pcie_ep()`, leading to potential kernel crashes on unpowered hardware access.
--

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

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

Previously, pci_pwrctrl_create_devices() was placed in imx_pcie_host_init(),
which caused it to be called multiple times across suspend/resume cycles.
This patch moves pci_pwrctrl_create_devices() to imx_pcie_probe() so that it
is only called once during probe.

> 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);
>  		if (ret < 0)
> -			return ret;
> +			goto err_pwrctrl_destroy;

[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() here?

Looking at imx_add_pcie_ep(), it calls imx_pcie_host_init(pp) but ignores
the integer error code it returns:

drivers/pci/controller/dwc/pci-imx6.c:imx_add_pcie_ep() {
    ...
    imx_pcie_host_init(pp);
    ep = &pci->ep;
    ep->ops = &pcie_ep_ops;
    ...
    ret = dw_pcie_ep_init(ep);
    if (ret) {
        dev_err(dev, "failed to initialize endpoint\n");
        return ret;
    }
    ...
}

If imx_pcie_host_init() fails to enable a clock or regulator, the hardware
is left unpowered and unclocked.

Since the function proceeds to call dw_pcie_ep_init() which reads and writes
to the controller's DBI registers, could this lead to a synchronous external
abort (bus error) and crash the entire system when it accesses the unclocked
peripheral registers?

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

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

[Sherry Sun]

> Subject: Re: [PATCH V6 3/4] Bluetooth: btnxpuart: Add M.2 Bluetooth device
> support using pwrseq
> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s)
> to consider:
> 
> New issues:
> - [High] Brittle logic causing probe failure on valid hardware topologies.
> 
> Pre-existing issues:
> - [High] Race condition exposing device before initializing synchronization
> primitives.
> - [High] Use-after-free and locking a destroyed mutex during device teardown.
> --
> 
> --- Patch [3]: [PATCH V6 3/4] Bluetooth: btnxpuart: Add M.2 Bluetooth device
> support using pwrseq --- commit
> 885432311fa71ac754d16037ae3e761e8192452c
> 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. This commit adds
> 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
> > --- 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 = pwrseq_get(&serdev->ctrl->dev, "uart");
> > +		if (IS_ERR(pwrseq))
> > +			return dev_err_probe(&serdev->dev,
> PTR_ERR(pwrseq),
> > +					     "failed to get pwrseq\n");
> 
> [Severity: High]
> If the parent UART node has an OF graph for other purposes, like audio
> routing or multiplexing, will this unconditionally fail the driver probe?
> 
> When of_graph_is_present() returns true, pwrseq_get() might return an error
> like -ENOENT if there is no power sequencer configured for the uart target.
> 
> Could this logic be made less brittle to support valid hardware topologies
> where the UART has an OF graph but no power sequencer?
> 

This is not an issue, for btnxpuart specifically, the OF graph link on the UART
controller node is only ever present to describe a BT device attached via the
M.2 Key E connector - that graph is the pwrseq link. There is currently no other
in-tree use of an OF graph on this controller node, so in every supported
topology, graph-present implies a pwrseq provider and pwrseq_get() matches.

A similar pattern can be found in drivers/pci/pwrctrl/generic.c, which also gates
the pwrseq path on of_graph_is_present() + dev_err_probe().

Best Regards
Sherry

> > +
> > +		nxpdev->pwrseq = pwrseq;
> > +		err = pwrseq_power_on(pwrseq);
> > +		if (err)
> > +			goto err_pwrseq_put;
> > +	}
> 
> [ ... ]
> 
> > @@ -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;
> > +	}
> 
> [Severity: High]
> This is a pre-existing issue, but does calling hci_register_dev() before
> ps_setup() create a race condition during initialization?
> 
> When hci_register_dev() is called, it schedules the power_on work
> asynchronously, which opens the UART port via btnxpuart_open().
> 
> Once the UART is open, incoming data triggers the receive path:
> 
> btnxpuart_receive_buf()
>   ps_wakeup()
> 
> ps_wakeup() attempts to lock psdata->ps_lock and schedule psdata->work.
> 
> If ps_setup() has not yet completed its initialization of these structures, could
> this result in operating on an uninitialized mutex or workqueue?
> 
> >
> >  	hci_devcd_register(hdev, nxp_coredump, nxp_coredump_hdr,
> 
> [ ... ]
> 
> > @@ -1952,6 +1976,8 @@ static void nxp_serdev_remove(struct
> serdev_device *serdev)
> >  		}
> >  	}
> >
> >  	ps_cleanup(nxpdev);
> >  	hci_unregister_dev(hdev);
> 
> [Severity: High]
> This is a pre-existing issue, but does calling ps_cleanup() before
> hci_unregister_dev() cause a use-after-free or a lock on a destroyed mutex?
> 
> ps_cleanup() destroys psdata->ps_lock and cancels psdata->work.
> 
> However, hci_unregister_dev() shuts down the device. During this shutdown
> process, the UART can still receive data or send commands.
> 
> If data is received or sent, the paths btnxpuart_receive_buf() to
> ps_wakeup() or nxp_enqueue() will attempt to acquire the destroyed
> psdata->ps_lock.
> 
> Should hci_unregister_dev() be called before cleaning up the synchronization
> primitives?
> 
> >  	reset_control_assert(nxpdev->pdn);
> > +	if (nxpdev->pwrseq)
> > +		pwrseq_put(nxpdev->pwrseq);
> >  	hci_free_dev(hdev);
> >  }
> 
> --
> Sashiko AI
> review * https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> Fsashiko.dev%2F%23%2Fpatchset%2F20260713023435.235765-1-
> sherry.sun%40oss.nxp.com%3Fpart%3D3&data=05%7C02%7Csherry.sun%40
> nxp.com%7C7fc8858da84f4a67665608dee088947e%7C686ea1d3bc2b4c6fa92
> cd99c5c301635%7C0%7C0%7C639195074371576326%7CUnknown%7CTWFp
> bGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4z
> MiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=xmwpxx%
> 2B1ofNIS08LXCuIRyjRW8g6PHY4iVOYOz6d9fg%3D&reserved=0

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

[Manivannan Sadhasivam]

On Mon, Jul 13, 2026 at 10:34:34AM +0800, Sherry Sun (OSS) wrote:
> 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 pwrseq_get() with
> the UART controller device (serdev->ctrl->dev), since the OF graph
> link is defined on the UART controller node.
> 
> Also add the explicit pwrseq_put() call in all exit paths, pwrseq_put()
> already calls pwrseq_power_off() internally, so no separate
> pwrseq_power_off() call is needed.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> Hi Frank and Bartosz, this V6 patch using pwrseq_put() in all exit paths to
> fix the power sequencer reference leak issue during BT driver unload and
> reload as reported by sashiko. 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..81a11ac05114 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 = 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)
> +			goto err_pwrseq_put;
> +	}
> +
>  	/* 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_put;
>  	}
>  
>  	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_put:
> +	if (nxpdev->pwrseq)
> +		pwrseq_put(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_put(nxpdev->pwrseq);
>  	hci_free_dev(hdev);
>  }
>  
> -- 
> 2.50.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: (subset) [PATCH V6 0/4] Add PCIe M.2 Key E connector support for NXP i.MX boards

[Manivannan Sadhasivam]

On Mon, 13 Jul 2026 10:34:31 +0800, Sherry Sun (OSS) wrote:
> 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.
> 
> [...]

Applied, thanks!

[1/4] PCI: imx6: Move pci_pwrctrl_create_devices() to imx_pcie_probe()
      commit: 2c5768344f88b86188c5915327bb00e6aad24990
[2/4] PCI: imx6: Add skip_pwrctrl_off flag support
      commit: f26b1c697a2bd72b1b68dca78ab1e95701a0ffb6

Best regards,
-- 
மணிவண்ணன் சதாசிவம்