Re: [PATCH v9 13/15] power: sequencing: pcie-m2: Split Bluetooth unit based on interface

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pwrseq: conflicting hardware state toggles between USB and UART
--

commit 93617e3cd28ec70ace0cc6829369a35a56e3ca15
Author: Chen-Yu Tsai <wenst@chromium.org>

power: sequencing: pcie-m2: Split Bluetooth unit based on interface

This commit splits the Bluetooth unit into two (USB and UART) to allow
either interface to independently toggle power, unbreaking the Qualcomm
Bluetooth serdev driver.

> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index 2066e3f1900df..bf5ebc2a82f53 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -83,8 +83,22 @@ static int pwrseq_pci_m2_e_bt_disable(struct pwrseq_device *pwrseq)
>  	return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 1);
>  }
>  
> -static const struct pwrseq_unit_data pwrseq_pcie_m2_e_bt_unit_data = {
> -	.name = "bt-enable",
> +/*
> + * XXX There are two Bluetooth units to allow either one to be able to power
> + * off and thus reset the controller. In practice only one of the interfaces
> + * is used, so there is no conflict. However userspace could power off the
> + * USB unit by disabling the associated USB port, without the UART unit or
> + * its consumer ever knowing.
> + */
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_bt_uart_unit_data = {
> +	.name = "bt-uart-enable",
> +	.deps = pwrseq_pcie_m2_unit_deps,
> +	.enable = pwrseq_pci_m2_e_bt_enable,
> +	.disable = pwrseq_pci_m2_e_bt_disable,
> +};
> +
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_bt_usb_unit_data = {
> +	.name = "bt-usb-enable",
>  	.deps = pwrseq_pcie_m2_unit_deps,
>  	.enable = pwrseq_pci_m2_e_bt_enable,
>  	.disable = pwrseq_pci_m2_e_bt_disable,
> +};

[Severity: Medium]
Will this bypass unified reference counting for the physical GPIO line?

If the USB subsystem (e.g., via sysfs port disable or fault recovery) disables
its unit, pwrseq_pci_m2_e_bt_disable() unconditionally drives the shared GPIO
to 1.

Could this forcefully reset the hardware and abruptly drop the active connection
of the UART driver, even if its reference count is still greater than 0?

Although the comments and commit message acknowledge this flaw, without an
exclusive lockout mechanism, does this expose the active subsystem to state
corruption from independent events and violate the PM consistency model?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916075745.3549953-1-wenst@chromium.org?part=13