[PATCH v4 5/5] power: sequencing: Add the Power Sequencing driver for the PCIe M.2 connectors

Manivannan Sadhasivam posted 5 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v4 5/5] power: sequencing: Add the Power Sequencing driver for the PCIe M.2 connectors
Posted by Manivannan Sadhasivam 1 month, 1 week ago
This driver is used to control the PCIe M.2 connectors of different
Mechanical Keys attached to the host machines and supporting different
interfaces like PCIe/SATA, USB/UART etc...

Currently, this driver supports only the Mechanical Key M connectors with
PCIe interface. The driver also only supports driving the mandatory 3.3v
and optional 1.8v power supplies. The optional signals of the Key M
connectors are not currently supported.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 MAINTAINERS                               |   7 ++
 drivers/power/sequencing/Kconfig          |   8 ++
 drivers/power/sequencing/Makefile         |   1 +
 drivers/power/sequencing/pwrseq-pcie-m2.c | 160 ++++++++++++++++++++++++++++++
 4 files changed, 176 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5b11839cba9d..2eb7b6d26573 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20791,6 +20791,13 @@ F:	Documentation/driver-api/pwrseq.rst
 F:	drivers/power/sequencing/
 F:	include/linux/pwrseq/
 
+PCIE M.2 POWER SEQUENCING
+M:	Manivannan Sadhasivam <mani@kernel.org>
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/connector/pcie-m2-m-connector.yaml
+F:	drivers/power/sequencing/pwrseq-pcie-m2.c
+
 POWER STATE COORDINATION INTERFACE (PSCI)
 M:	Mark Rutland <mark.rutland@arm.com>
 M:	Lorenzo Pieralisi <lpieralisi@kernel.org>
diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
index 280f92beb5d0..f5fff84566ba 100644
--- a/drivers/power/sequencing/Kconfig
+++ b/drivers/power/sequencing/Kconfig
@@ -35,4 +35,12 @@ config POWER_SEQUENCING_TH1520_GPU
 	  GPU. This driver handles the complex clock and reset sequence
 	  required to power on the Imagination BXM GPU on this platform.
 
+config POWER_SEQUENCING_PCIE_M2
+	tristate "PCIe M.2 connector power sequencing driver"
+	depends on OF || COMPILE_TEST
+	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
+	  connectors exposing multiple interfaces like PCIe, SATA, UART, etc...
+
 endif
diff --git a/drivers/power/sequencing/Makefile b/drivers/power/sequencing/Makefile
index 96c1cf0a98ac..0911d4618298 100644
--- a/drivers/power/sequencing/Makefile
+++ b/drivers/power/sequencing/Makefile
@@ -5,3 +5,4 @@ pwrseq-core-y				:= core.o
 
 obj-$(CONFIG_POWER_SEQUENCING_QCOM_WCN)	+= pwrseq-qcom-wcn.o
 obj-$(CONFIG_POWER_SEQUENCING_TH1520_GPU) += pwrseq-thead-gpu.o
+obj-$(CONFIG_POWER_SEQUENCING_PCIE_M2)	+= pwrseq-pcie-m2.o
diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
new file mode 100644
index 000000000000..4835d099d967
--- /dev/null
+++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ * Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
+ */
+
+#include <linux/device.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/platform_device.h>
+#include <linux/pwrseq/provider.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+struct pwrseq_pcie_m2_pdata {
+	const struct pwrseq_target_data **targets;
+};
+
+struct pwrseq_pcie_m2_ctx {
+	struct pwrseq_device *pwrseq;
+	struct device_node *of_node;
+	const struct pwrseq_pcie_m2_pdata *pdata;
+	struct regulator_bulk_data *regs;
+	size_t num_vregs;
+	struct notifier_block nb;
+};
+
+static int pwrseq_pcie_m2_m_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)
+{
+	struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+
+	return regulator_bulk_disable(ctx->num_vregs, ctx->regs);
+}
+
+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,
+};
+
+static const struct pwrseq_unit_data *pwrseq_pcie_m2_m_unit_deps[] = {
+	&pwrseq_pcie_m2_vregs_unit_data,
+	NULL
+};
+
+static const struct pwrseq_unit_data pwrseq_pcie_m2_m_pcie_unit_data = {
+	.name = "pcie-enable",
+	.deps = pwrseq_pcie_m2_m_unit_deps,
+};
+
+static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
+	.name = "pcie",
+	.unit = &pwrseq_pcie_m2_m_pcie_unit_data,
+};
+
+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_m_of_data = {
+	.targets = pwrseq_pcie_m2_m_targets,
+};
+
+static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
+				 struct device *dev)
+{
+	struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
+	struct device_node *endpoint __free(device_node) = NULL;
+
+	/*
+	 * Traverse the 'remote-endpoint' nodes and check if the remote node's
+	 * parent matches the OF node of 'dev'.
+	 */
+	for_each_endpoint_of_node(ctx->of_node, endpoint) {
+		struct device_node *remote __free(device_node) =
+				of_graph_get_remote_port_parent(endpoint);
+		if (remote && (remote == dev_of_node(dev)))
+			return PWRSEQ_MATCH_OK;
+	}
+
+	return PWRSEQ_NO_MATCH;
+}
+
+static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct pwrseq_pcie_m2_ctx *ctx;
+	struct pwrseq_config config = {};
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->of_node = dev_of_node(dev);
+	ctx->pdata = device_get_match_data(dev);
+	if (!ctx->pdata)
+		return dev_err_probe(dev, -ENODEV,
+				     "Failed to obtain platform data\n");
+
+	/*
+	 * Currently, of_regulator_bulk_get_all() is the only regulator API that
+	 * allows to get all supplies in the devicetree node without manually
+	 * specifying them.
+	 */
+	ret = of_regulator_bulk_get_all(dev, dev_of_node(dev), &ctx->regs);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "Failed to get all regulators\n");
+
+	ctx->num_vregs = ret;
+
+	config.parent = dev;
+	config.owner = THIS_MODULE;
+	config.drvdata = ctx;
+	config.match = pwrseq_pcie_m2_match;
+	config.targets = ctx->pdata->targets;
+
+	ctx->pwrseq = devm_pwrseq_device_register(dev, &config);
+	if (IS_ERR(ctx->pwrseq)) {
+		regulator_bulk_free(ctx->num_vregs, ctx->regs);
+		return dev_err_probe(dev, PTR_ERR(ctx->pwrseq),
+				     "Failed to register the power sequencer\n");
+	}
+
+	return 0;
+}
+
+static const struct of_device_id pwrseq_pcie_m2_of_match[] = {
+	{
+		.compatible = "pcie-m2-m-connector",
+		.data = &pwrseq_pcie_m2_m_of_data,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pwrseq_pcie_m2_of_match);
+
+static struct platform_driver pwrseq_pcie_m2_driver = {
+	.driver = {
+		.name = "pwrseq-pcie-m2",
+		.of_match_table = pwrseq_pcie_m2_of_match,
+	},
+	.probe = pwrseq_pcie_m2_probe,
+};
+module_platform_driver(pwrseq_pcie_m2_driver);
+
+MODULE_AUTHOR("Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>");
+MODULE_DESCRIPTION("Power Sequencing driver for PCIe M.2 connector");
+MODULE_LICENSE("GPL");

-- 
2.48.1
Re: [PATCH v4 5/5] power: sequencing: Add the Power Sequencing driver for the PCIe M.2 connectors
Posted by Bartosz Golaszewski 1 month, 1 week ago
On Sun, Dec 28, 2025 at 6:01 PM Manivannan Sadhasivam
<manivannan.sadhasivam@oss.qualcomm.com> wrote:
>
> This driver is used to control the PCIe M.2 connectors of different
> Mechanical Keys attached to the host machines and supporting different
> interfaces like PCIe/SATA, USB/UART etc...
>
> Currently, this driver supports only the Mechanical Key M connectors with
> PCIe interface. The driver also only supports driving the mandatory 3.3v
> and optional 1.8v power supplies. The optional signals of the Key M
> connectors are not currently supported.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  MAINTAINERS                               |   7 ++
>  drivers/power/sequencing/Kconfig          |   8 ++
>  drivers/power/sequencing/Makefile         |   1 +
>  drivers/power/sequencing/pwrseq-pcie-m2.c | 160 ++++++++++++++++++++++++++++++
>  4 files changed, 176 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5b11839cba9d..2eb7b6d26573 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20791,6 +20791,13 @@ F:     Documentation/driver-api/pwrseq.rst
>  F:     drivers/power/sequencing/
>  F:     include/linux/pwrseq/
>
> +PCIE M.2 POWER SEQUENCING
> +M:     Manivannan Sadhasivam <mani@kernel.org>
> +L:     linux-pci@vger.kernel.org
> +S:     Maintained
> +F:     Documentation/devicetree/bindings/connector/pcie-m2-m-connector.yaml
> +F:     drivers/power/sequencing/pwrseq-pcie-m2.c
> +
>  POWER STATE COORDINATION INTERFACE (PSCI)
>  M:     Mark Rutland <mark.rutland@arm.com>
>  M:     Lorenzo Pieralisi <lpieralisi@kernel.org>
> diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> index 280f92beb5d0..f5fff84566ba 100644
> --- a/drivers/power/sequencing/Kconfig
> +++ b/drivers/power/sequencing/Kconfig
> @@ -35,4 +35,12 @@ config POWER_SEQUENCING_TH1520_GPU
>           GPU. This driver handles the complex clock and reset sequence
>           required to power on the Imagination BXM GPU on this platform.
>
> +config POWER_SEQUENCING_PCIE_M2
> +       tristate "PCIe M.2 connector power sequencing driver"
> +       depends on OF || COMPILE_TEST
> +       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
> +         connectors exposing multiple interfaces like PCIe, SATA, UART, etc...
> +
>  endif
> diff --git a/drivers/power/sequencing/Makefile b/drivers/power/sequencing/Makefile
> index 96c1cf0a98ac..0911d4618298 100644
> --- a/drivers/power/sequencing/Makefile
> +++ b/drivers/power/sequencing/Makefile
> @@ -5,3 +5,4 @@ pwrseq-core-y                           := core.o
>
>  obj-$(CONFIG_POWER_SEQUENCING_QCOM_WCN)        += pwrseq-qcom-wcn.o
>  obj-$(CONFIG_POWER_SEQUENCING_TH1520_GPU) += pwrseq-thead-gpu.o
> +obj-$(CONFIG_POWER_SEQUENCING_PCIE_M2) += pwrseq-pcie-m2.o
> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> new file mode 100644
> index 000000000000..4835d099d967
> --- /dev/null
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + * Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> + */
> +
> +#include <linux/device.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/platform_device.h>
> +#include <linux/pwrseq/provider.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +struct pwrseq_pcie_m2_pdata {
> +       const struct pwrseq_target_data **targets;
> +};
> +
> +struct pwrseq_pcie_m2_ctx {
> +       struct pwrseq_device *pwrseq;
> +       struct device_node *of_node;
> +       const struct pwrseq_pcie_m2_pdata *pdata;
> +       struct regulator_bulk_data *regs;
> +       size_t num_vregs;
> +       struct notifier_block nb;
> +};
> +
> +static int pwrseq_pcie_m2_m_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)
> +{
> +       struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> +       return regulator_bulk_disable(ctx->num_vregs, ctx->regs);
> +}
> +
> +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,
> +};
> +
> +static const struct pwrseq_unit_data *pwrseq_pcie_m2_m_unit_deps[] = {
> +       &pwrseq_pcie_m2_vregs_unit_data,
> +       NULL
> +};
> +
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_m_pcie_unit_data = {
> +       .name = "pcie-enable",
> +       .deps = pwrseq_pcie_m2_m_unit_deps,
> +};
> +
> +static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
> +       .name = "pcie",
> +       .unit = &pwrseq_pcie_m2_m_pcie_unit_data,
> +};
> +
> +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_m_of_data = {
> +       .targets = pwrseq_pcie_m2_m_targets,
> +};
> +
> +static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
> +                                struct device *dev)
> +{
> +       struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +       struct device_node *endpoint __free(device_node) = NULL;
> +
> +       /*
> +        * Traverse the 'remote-endpoint' nodes and check if the remote node's
> +        * parent matches the OF node of 'dev'.
> +        */
> +       for_each_endpoint_of_node(ctx->of_node, endpoint) {
> +               struct device_node *remote __free(device_node) =
> +                               of_graph_get_remote_port_parent(endpoint);
> +               if (remote && (remote == dev_of_node(dev)))
> +                       return PWRSEQ_MATCH_OK;
> +       }
> +
> +       return PWRSEQ_NO_MATCH;
> +}
> +
> +static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct pwrseq_pcie_m2_ctx *ctx;
> +       struct pwrseq_config config = {};
> +       int ret;
> +
> +       ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +       if (!ctx)
> +               return -ENOMEM;
> +
> +       ctx->of_node = dev_of_node(dev);

Since you're storing the node address for later, I'd suggest using
of_node_get() to get a real reference.

> +       ctx->pdata = device_get_match_data(dev);
> +       if (!ctx->pdata)
> +               return dev_err_probe(dev, -ENODEV,
> +                                    "Failed to obtain platform data\n");
> +
> +       /*
> +        * Currently, of_regulator_bulk_get_all() is the only regulator API that
> +        * allows to get all supplies in the devicetree node without manually
> +        * specifying them.
> +        */
> +       ret = of_regulator_bulk_get_all(dev, dev_of_node(dev), &ctx->regs);
> +       if (ret < 0)
> +               return dev_err_probe(dev, ret,
> +                                    "Failed to get all regulators\n");
> +
> +       ctx->num_vregs = ret;
> +
> +       config.parent = dev;
> +       config.owner = THIS_MODULE;
> +       config.drvdata = ctx;
> +       config.match = pwrseq_pcie_m2_match;
> +       config.targets = ctx->pdata->targets;
> +
> +       ctx->pwrseq = devm_pwrseq_device_register(dev, &config);
> +       if (IS_ERR(ctx->pwrseq)) {
> +               regulator_bulk_free(ctx->num_vregs, ctx->regs);

You're freeing it on error but not on driver detach? Maybe schedule a
devm action if there's no devres variant?

> +               return dev_err_probe(dev, PTR_ERR(ctx->pwrseq),
> +                                    "Failed to register the power sequencer\n");
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id pwrseq_pcie_m2_of_match[] = {
> +       {
> +               .compatible = "pcie-m2-m-connector",
> +               .data = &pwrseq_pcie_m2_m_of_data,
> +       },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, pwrseq_pcie_m2_of_match);
> +
> +static struct platform_driver pwrseq_pcie_m2_driver = {
> +       .driver = {
> +               .name = "pwrseq-pcie-m2",
> +               .of_match_table = pwrseq_pcie_m2_of_match,
> +       },
> +       .probe = pwrseq_pcie_m2_probe,
> +};
> +module_platform_driver(pwrseq_pcie_m2_driver);
> +
> +MODULE_AUTHOR("Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>");
> +MODULE_DESCRIPTION("Power Sequencing driver for PCIe M.2 connector");
> +MODULE_LICENSE("GPL");
>
> --
> 2.48.1
>

Bart
Re: [PATCH v4 5/5] power: sequencing: Add the Power Sequencing driver for the PCIe M.2 connectors
Posted by Manivannan Sadhasivam 1 month ago
On Fri, Jan 02, 2026 at 12:26:21PM +0100, Bartosz Golaszewski wrote:
> On Sun, Dec 28, 2025 at 6:01 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@oss.qualcomm.com> wrote:
> >
> > This driver is used to control the PCIe M.2 connectors of different
> > Mechanical Keys attached to the host machines and supporting different
> > interfaces like PCIe/SATA, USB/UART etc...
> >
> > Currently, this driver supports only the Mechanical Key M connectors with
> > PCIe interface. The driver also only supports driving the mandatory 3.3v
> > and optional 1.8v power supplies. The optional signals of the Key M
> > connectors are not currently supported.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  MAINTAINERS                               |   7 ++
> >  drivers/power/sequencing/Kconfig          |   8 ++
> >  drivers/power/sequencing/Makefile         |   1 +
> >  drivers/power/sequencing/pwrseq-pcie-m2.c | 160 ++++++++++++++++++++++++++++++
> >  4 files changed, 176 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5b11839cba9d..2eb7b6d26573 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -20791,6 +20791,13 @@ F:     Documentation/driver-api/pwrseq.rst
> >  F:     drivers/power/sequencing/
> >  F:     include/linux/pwrseq/
> >
> > +PCIE M.2 POWER SEQUENCING
> > +M:     Manivannan Sadhasivam <mani@kernel.org>
> > +L:     linux-pci@vger.kernel.org
> > +S:     Maintained
> > +F:     Documentation/devicetree/bindings/connector/pcie-m2-m-connector.yaml
> > +F:     drivers/power/sequencing/pwrseq-pcie-m2.c
> > +
> >  POWER STATE COORDINATION INTERFACE (PSCI)
> >  M:     Mark Rutland <mark.rutland@arm.com>
> >  M:     Lorenzo Pieralisi <lpieralisi@kernel.org>
> > diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> > index 280f92beb5d0..f5fff84566ba 100644
> > --- a/drivers/power/sequencing/Kconfig
> > +++ b/drivers/power/sequencing/Kconfig
> > @@ -35,4 +35,12 @@ config POWER_SEQUENCING_TH1520_GPU
> >           GPU. This driver handles the complex clock and reset sequence
> >           required to power on the Imagination BXM GPU on this platform.
> >
> > +config POWER_SEQUENCING_PCIE_M2
> > +       tristate "PCIe M.2 connector power sequencing driver"
> > +       depends on OF || COMPILE_TEST
> > +       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
> > +         connectors exposing multiple interfaces like PCIe, SATA, UART, etc...
> > +
> >  endif
> > diff --git a/drivers/power/sequencing/Makefile b/drivers/power/sequencing/Makefile
> > index 96c1cf0a98ac..0911d4618298 100644
> > --- a/drivers/power/sequencing/Makefile
> > +++ b/drivers/power/sequencing/Makefile
> > @@ -5,3 +5,4 @@ pwrseq-core-y                           := core.o
> >
> >  obj-$(CONFIG_POWER_SEQUENCING_QCOM_WCN)        += pwrseq-qcom-wcn.o
> >  obj-$(CONFIG_POWER_SEQUENCING_TH1520_GPU) += pwrseq-thead-gpu.o
> > +obj-$(CONFIG_POWER_SEQUENCING_PCIE_M2) += pwrseq-pcie-m2.o
> > diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> > new file mode 100644
> > index 000000000000..4835d099d967
> > --- /dev/null
> > +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> > @@ -0,0 +1,160 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + * Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > + */
> > +
> > +#include <linux/device.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/platform_device.h>
> > +#include <linux/pwrseq/provider.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/slab.h>
> > +
> > +struct pwrseq_pcie_m2_pdata {
> > +       const struct pwrseq_target_data **targets;
> > +};
> > +
> > +struct pwrseq_pcie_m2_ctx {
> > +       struct pwrseq_device *pwrseq;
> > +       struct device_node *of_node;
> > +       const struct pwrseq_pcie_m2_pdata *pdata;
> > +       struct regulator_bulk_data *regs;
> > +       size_t num_vregs;
> > +       struct notifier_block nb;
> > +};
> > +
> > +static int pwrseq_pcie_m2_m_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)
> > +{
> > +       struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> > +
> > +       return regulator_bulk_disable(ctx->num_vregs, ctx->regs);
> > +}
> > +
> > +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,
> > +};
> > +
> > +static const struct pwrseq_unit_data *pwrseq_pcie_m2_m_unit_deps[] = {
> > +       &pwrseq_pcie_m2_vregs_unit_data,
> > +       NULL
> > +};
> > +
> > +static const struct pwrseq_unit_data pwrseq_pcie_m2_m_pcie_unit_data = {
> > +       .name = "pcie-enable",
> > +       .deps = pwrseq_pcie_m2_m_unit_deps,
> > +};
> > +
> > +static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
> > +       .name = "pcie",
> > +       .unit = &pwrseq_pcie_m2_m_pcie_unit_data,
> > +};
> > +
> > +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_m_of_data = {
> > +       .targets = pwrseq_pcie_m2_m_targets,
> > +};
> > +
> > +static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
> > +                                struct device *dev)
> > +{
> > +       struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> > +       struct device_node *endpoint __free(device_node) = NULL;
> > +
> > +       /*
> > +        * Traverse the 'remote-endpoint' nodes and check if the remote node's
> > +        * parent matches the OF node of 'dev'.
> > +        */
> > +       for_each_endpoint_of_node(ctx->of_node, endpoint) {
> > +               struct device_node *remote __free(device_node) =
> > +                               of_graph_get_remote_port_parent(endpoint);
> > +               if (remote && (remote == dev_of_node(dev)))
> > +                       return PWRSEQ_MATCH_OK;
> > +       }
> > +
> > +       return PWRSEQ_NO_MATCH;
> > +}
> > +
> > +static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct pwrseq_pcie_m2_ctx *ctx;
> > +       struct pwrseq_config config = {};
> > +       int ret;
> > +
> > +       ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > +       if (!ctx)
> > +               return -ENOMEM;
> > +
> > +       ctx->of_node = dev_of_node(dev);
> 
> Since you're storing the node address for later, I'd suggest using
> of_node_get() to get a real reference.
> 

If CONFIG_OF_DYNAMIC is not enabled, then of_node_get() will just return the
passed pointer. I always prefer using dev_of_node() since it has the CONFIG_OF
and NULL check. Though, the checks won't apply here, I used it for consistency.

> > +       ctx->pdata = device_get_match_data(dev);
> > +       if (!ctx->pdata)
> > +               return dev_err_probe(dev, -ENODEV,
> > +                                    "Failed to obtain platform data\n");
> > +
> > +       /*
> > +        * Currently, of_regulator_bulk_get_all() is the only regulator API that
> > +        * allows to get all supplies in the devicetree node without manually
> > +        * specifying them.
> > +        */
> > +       ret = of_regulator_bulk_get_all(dev, dev_of_node(dev), &ctx->regs);
> > +       if (ret < 0)
> > +               return dev_err_probe(dev, ret,
> > +                                    "Failed to get all regulators\n");
> > +
> > +       ctx->num_vregs = ret;
> > +
> > +       config.parent = dev;
> > +       config.owner = THIS_MODULE;
> > +       config.drvdata = ctx;
> > +       config.match = pwrseq_pcie_m2_match;
> > +       config.targets = ctx->pdata->targets;
> > +
> > +       ctx->pwrseq = devm_pwrseq_device_register(dev, &config);
> > +       if (IS_ERR(ctx->pwrseq)) {
> > +               regulator_bulk_free(ctx->num_vregs, ctx->regs);
> 
> You're freeing it on error but not on driver detach? Maybe schedule a
> devm action if there's no devres variant?
> 

Ok!

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v4 5/5] power: sequencing: Add the Power Sequencing driver for the PCIe M.2 connectors
Posted by Bartosz Golaszewski 1 month ago
On Wed, Jan 7, 2026 at 10:39 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> > > +
> > > +static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> > > +{
> > > +       struct device *dev = &pdev->dev;
> > > +       struct pwrseq_pcie_m2_ctx *ctx;
> > > +       struct pwrseq_config config = {};
> > > +       int ret;
> > > +
> > > +       ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > > +       if (!ctx)
> > > +               return -ENOMEM;
> > > +
> > > +       ctx->of_node = dev_of_node(dev);
> >
> > Since you're storing the node address for later, I'd suggest using
> > of_node_get() to get a real reference.
> >
>
> If CONFIG_OF_DYNAMIC is not enabled, then of_node_get() will just return the
> passed pointer. I always prefer using dev_of_node() since it has the CONFIG_OF
> and NULL check. Though, the checks won't apply here, I used it for consistency.
>

I think it's just more of a good practice to take a reference to any
resource whenever you store keep it for longer than the duration of
the function even if the actual reference counting is disabled in some
instances. If ever we switch to fwnodes, the circumstances may be
different than static devicetree.

You can also do "ctx->of_node = of_node_get(dev_of_node(dev));", all
the NULL-checks are there.

Bart
Re: [PATCH v4 5/5] power: sequencing: Add the Power Sequencing driver for the PCIe M.2 connectors
Posted by Manivannan Sadhasivam 1 month ago
On Wed, Jan 07, 2026 at 10:51:11AM +0100, Bartosz Golaszewski wrote:
> On Wed, Jan 7, 2026 at 10:39 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > > > +
> > > > +static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> > > > +{
> > > > +       struct device *dev = &pdev->dev;
> > > > +       struct pwrseq_pcie_m2_ctx *ctx;
> > > > +       struct pwrseq_config config = {};
> > > > +       int ret;
> > > > +
> > > > +       ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > > > +       if (!ctx)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       ctx->of_node = dev_of_node(dev);
> > >
> > > Since you're storing the node address for later, I'd suggest using
> > > of_node_get() to get a real reference.
> > >
> >
> > If CONFIG_OF_DYNAMIC is not enabled, then of_node_get() will just return the
> > passed pointer. I always prefer using dev_of_node() since it has the CONFIG_OF
> > and NULL check. Though, the checks won't apply here, I used it for consistency.
> >
> 
> I think it's just more of a good practice to take a reference to any
> resource whenever you store keep it for longer than the duration of
> the function even if the actual reference counting is disabled in some
> instances.

Good practice you inherited from writing Rust code :)

> If ever we switch to fwnodes, the circumstances may be
> different than static devicetree.
> 
> You can also do "ctx->of_node = of_node_get(dev_of_node(dev));", all
> the NULL-checks are there.
> 

This may not be needed. I can use of_node_get() here, but the APIs are just
fragile such that neither dev_of_node() nor of_node_get() increments the
refcount always.

- Mani

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