| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <z33axfsiox73f2lklhiaulekjnqxnqtkycfylybwqnqxtx2fck@3qtas4u6mfnz>
Date: Wed, 7 Jan 2026 15:08:55 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>,
Bjorn Helgaas <bhelgaas@...gle.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Damien Le Moal <dlemoal@...nel.org>, Niklas Cassel <cassel@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>, linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
Stephan Gerhold <stephan.gerhold@...aro.org>, Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
linux-pm@...r.kernel.org, linux-ide@...r.kernel.org
Subject: Re: [PATCH v4 5/5] power: sequencing: Add the Power Sequencing
driver for the PCIe M.2 connectors
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@....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@....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@...nel.org>
> > +L: linux-pci@...r.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@....com>
> > M: Lorenzo Pieralisi <lpieralisi@...nel.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@....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
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists