Re: [PATCH v4 8/9] power: sequencing: pcie-m2: Add support for PCIe M.2 Key E connectors

[Sean Anderson <sean.anderson@linux.dev>]

On 1/12/26 11:26, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> Add support for handling the power sequence of the PCIe M.2 Key E
> connectors. These connectors are used to attach the Wireless Connectivity
> devices to the host machine including combinations of WiFi, BT, NFC using
> interfaces such as PCIe/SDIO for WiFi, USB/UART for BT and I2C for NFC.
> 
> Currently, this driver supports only the PCIe interface for WiFi and UART
> interface for BT. The driver also only supports driving the 3.3v/1.8v power
> supplies and W_DISABLE{1/2}# GPIOs. The optional signals of the Key E
> connectors are not currently supported.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/power/sequencing/Kconfig          |   1 +
>  drivers/power/sequencing/pwrseq-pcie-m2.c | 110 ++++++++++++++++++++++++++++--
>  2 files changed, 104 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> index f5fff84566ba..29bd204319cc 100644
> --- a/drivers/power/sequencing/Kconfig
> +++ b/drivers/power/sequencing/Kconfig
> @@ -38,6 +38,7 @@ config POWER_SEQUENCING_TH1520_GPU
>  config POWER_SEQUENCING_PCIE_M2
>  	tristate "PCIe M.2 connector power sequencing driver"
>  	depends on OF || COMPILE_TEST
> +	depends on PCI
>  	help
>  	  Say Y here to enable the power sequencing driver for PCIe M.2
>  	  connectors. This driver handles the power sequencing for the M.2
> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index e01e19123415..4b85a40d7692 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -4,12 +4,16 @@
>   * Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>   */
>  
> +#include <linux/err.h>
>  #include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_graph.h>
>  #include <linux/of_platform.h>
> +#include <linux/pci.h>
>  #include <linux/platform_device.h>
>  #include <linux/pwrseq/provider.h>
>  #include <linux/regulator/consumer.h>
> @@ -25,17 +29,19 @@ struct pwrseq_pcie_m2_ctx {
>  	const struct pwrseq_pcie_m2_pdata *pdata;
>  	struct regulator_bulk_data *regs;
>  	size_t num_vregs;
> -	struct notifier_block nb;
> +	struct gpio_desc *w_disable1_gpio;
> +	struct gpio_desc *w_disable2_gpio;
> +	struct device *dev;
>  };
>  
> -static int pwrseq_pcie_m2_m_vregs_enable(struct pwrseq_device *pwrseq)
> +static int pwrseq_pcie_m2_vregs_enable(struct pwrseq_device *pwrseq)
>  {
>  	struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
>  
>  	return regulator_bulk_enable(ctx->num_vregs, ctx->regs);
>  }
>  
> -static int pwrseq_pcie_m2_m_vregs_disable(struct pwrseq_device *pwrseq)
> +static int pwrseq_pcie_m2_vregs_disable(struct pwrseq_device *pwrseq)
>  {
>  	struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
>  
> @@ -44,18 +50,84 @@ static int pwrseq_pcie_m2_m_vregs_disable(struct pwrseq_device *pwrseq)
>  
>  static const struct pwrseq_unit_data pwrseq_pcie_m2_vregs_unit_data = {
>  	.name = "regulators-enable",
> -	.enable = pwrseq_pcie_m2_m_vregs_enable,
> -	.disable = pwrseq_pcie_m2_m_vregs_disable,
> +	.enable = pwrseq_pcie_m2_vregs_enable,
> +	.disable = pwrseq_pcie_m2_vregs_disable,
>  };
>  
> -static const struct pwrseq_unit_data *pwrseq_pcie_m2_m_unit_deps[] = {
> +static const struct pwrseq_unit_data *pwrseq_pcie_m2_unit_deps[] = {
>  	&pwrseq_pcie_m2_vregs_unit_data,
>  	NULL
>  };
>  
> +static int pwrseq_pci_m2_e_uart_enable(struct pwrseq_device *pwrseq)
> +{
> +	struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> +	return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 0);
> +}
> +
> +static int pwrseq_pci_m2_e_uart_disable(struct pwrseq_device *pwrseq)
> +{
> +	struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> +	return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 1);
> +}
> +
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_uart_unit_data = {
> +	.name = "uart-enable",
> +	.deps = pwrseq_pcie_m2_unit_deps,
> +	.enable = pwrseq_pci_m2_e_uart_enable,
> +	.disable = pwrseq_pci_m2_e_uart_disable,
> +};
> +
> +static int pwrseq_pci_m2_e_pcie_enable(struct pwrseq_device *pwrseq)
> +{
> +	struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> +	return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 0);
> +}
> +
> +static int pwrseq_pci_m2_e_pcie_disable(struct pwrseq_device *pwrseq)
> +{
> +	struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> +	return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 1);
> +}
> +
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_pcie_unit_data = {
> +	.name = "pcie-enable",
> +	.deps = pwrseq_pcie_m2_unit_deps,
> +	.enable = pwrseq_pci_m2_e_pcie_enable,
> +	.disable = pwrseq_pci_m2_e_pcie_disable,
> +};
> +
>  static const struct pwrseq_unit_data pwrseq_pcie_m2_m_pcie_unit_data = {
>  	.name = "pcie-enable",
> -	.deps = pwrseq_pcie_m2_m_unit_deps,
> +	.deps = pwrseq_pcie_m2_unit_deps,
> +};
> +
> +static int pwrseq_pcie_m2_e_pwup_delay(struct pwrseq_device *pwrseq)
> +{
> +	/*
> +	 * FIXME: This delay is only required for some Qcom WLAN/BT cards like
> +	 * WCN7850 and not for all devices. But currently, there is no way to
> +	 * identify the device model before enumeration.
> +	 */
> +	msleep(50);

Section 3.1.4 of the M.2 spec says that "Power Valid to PERST# input
inactive" (T_PVPGL) is "Implementation specific recommended 50 ms." So I
think we should delay for at least 50 ms for all M.2 cards.
Additionally, the PCIe CEM specifies that "Power stable to PERST#
inactive" (T_PVPERL) must be at least 100 ms. So I think we should just
delay for 100 ms regardless of the slot, perhaps making this
configurable in the devicetree if e.g. the system integrator knows the
soldered-down M.2 requires less initialization time. This is exactly
what I proposed in [1].

--Sean

[1] https://lore.kernel.org/linux-pci/20251219172222.2808195-2-sean.anderson@linux.dev/

> +	return 0;
> +}
> +
> +static const struct pwrseq_target_data pwrseq_pcie_m2_e_uart_target_data = {
> +	.name = "uart",
> +	.unit = &pwrseq_pcie_m2_e_uart_unit_data,
> +	.post_enable = pwrseq_pcie_m2_e_pwup_delay,
> +};
> +
> +static const struct pwrseq_target_data pwrseq_pcie_m2_e_pcie_target_data = {
> +	.name = "pcie",
> +	.unit = &pwrseq_pcie_m2_e_pcie_unit_data,
> +	.post_enable = pwrseq_pcie_m2_e_pwup_delay,
>  };
>  
>  static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
> @@ -63,11 +135,21 @@ static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
>  	.unit = &pwrseq_pcie_m2_m_pcie_unit_data,
>  };
>  
> +static const struct pwrseq_target_data *pwrseq_pcie_m2_e_targets[] = {
> +	&pwrseq_pcie_m2_e_pcie_target_data,
> +	&pwrseq_pcie_m2_e_uart_target_data,
> +	NULL
> +};
> +
>  static const struct pwrseq_target_data *pwrseq_pcie_m2_m_targets[] = {
>  	&pwrseq_pcie_m2_m_pcie_target_data,
>  	NULL
>  };
>  
> +static const struct pwrseq_pcie_m2_pdata pwrseq_pcie_m2_e_of_data = {
> +	.targets = pwrseq_pcie_m2_e_targets,
> +};
> +
>  static const struct pwrseq_pcie_m2_pdata pwrseq_pcie_m2_m_of_data = {
>  	.targets = pwrseq_pcie_m2_m_targets,
>  };
> @@ -126,6 +208,16 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, ret,
>  				     "Failed to get all regulators\n");
>  
> +	ctx->w_disable1_gpio = devm_gpiod_get_optional(dev, "w-disable1", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ctx->w_disable1_gpio))
> +		return dev_err_probe(dev, PTR_ERR(ctx->w_disable1_gpio),
> +				     "Failed to get the W_DISABLE_1# GPIO\n");
> +
> +	ctx->w_disable2_gpio = devm_gpiod_get_optional(dev, "w-disable2", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ctx->w_disable2_gpio))
> +		return dev_err_probe(dev, PTR_ERR(ctx->w_disable2_gpio),
> +				     "Failed to get the W_DISABLE_2# GPIO\n");
> +
>  	ctx->num_vregs = ret;
>  
>  	ret = devm_add_action_or_reset(dev, pwrseq_pcie_free_resources, ctx);
> @@ -151,6 +243,10 @@ static const struct of_device_id pwrseq_pcie_m2_of_match[] = {
>  		.compatible = "pcie-m2-m-connector",
>  		.data = &pwrseq_pcie_m2_m_of_data,
>  	},
> +	{
> +		.compatible = "pcie-m2-e-connector",
> +		.data = &pwrseq_pcie_m2_e_of_data,
> +	},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, pwrseq_pcie_m2_of_match);
>