[PATCH v6 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth

Manivannan Sadhasivam via B4 Relay posted 9 patches 2 weeks, 6 days ago
There is a newer version of this series
[PATCH v6 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth
Posted by Manivannan Sadhasivam via B4 Relay 2 weeks, 6 days ago
From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

For supporting bluetooth over the non-discoverable UART interface of
WCN7850, create the serdev device after enumerating the PCIe interface.
This is mandatory since the device ID is only known after the PCIe
enumeration and the ID is used for creating the serdev device.

Since by default there is no OF or ACPI node for the created serdev,
create a dynamic OF 'bluetooth' node with the 'compatible' property and
attach it to the serdev device. This will allow the serdev device to bind
to the existing bluetooth driver.

Tested-by: Hans de Goede <johannes.goede@oss.qualcomm.com> # ThinkPad T14s gen6 (arm64)
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/power/sequencing/Kconfig          |   3 +-
 drivers/power/sequencing/pwrseq-pcie-m2.c | 178 +++++++++++++++++++++++++++++-
 2 files changed, 177 insertions(+), 4 deletions(-)

diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
index f5fff84566ba..55aeef125e6f 100644
--- a/drivers/power/sequencing/Kconfig
+++ b/drivers/power/sequencing/Kconfig
@@ -37,7 +37,8 @@ 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 && OF) || COMPILE_TEST
+	select OF_DYNAMIC
 	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 3507cdcb1e7b..77357439ba81 100644
--- a/drivers/power/sequencing/pwrseq-pcie-m2.c
+++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
@@ -12,9 +12,11 @@
 #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>
+#include <linux/serdev.h>
 #include <linux/slab.h>
 
 struct pwrseq_pcie_m2_pdata {
@@ -30,6 +32,9 @@ struct pwrseq_pcie_m2_ctx {
 	struct notifier_block nb;
 	struct gpio_desc *w_disable1_gpio;
 	struct gpio_desc *w_disable2_gpio;
+	struct serdev_device *serdev;
+	struct of_changeset *ocs;
+	struct device *dev;
 };
 
 static int pwrseq_pcie_m2_vregs_enable(struct pwrseq_device *pwrseq)
@@ -172,13 +177,176 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
 	return PWRSEQ_NO_MATCH;
 }
 
-static void pwrseq_pcie_m2_free_regulators(void *data)
+static void pwrseq_pcie_m2_free_resources(void *data)
 {
 	struct pwrseq_pcie_m2_ctx *ctx = data;
 
+	serdev_device_remove(ctx->serdev);
+	bus_unregister_notifier(&pci_bus_type, &ctx->nb);
+	of_changeset_revert(ctx->ocs);
+	of_changeset_destroy(ctx->ocs);
 	regulator_bulk_free(ctx->num_vregs, ctx->regs);
 }
 
+static int pwrseq_m2_pcie_create_bt_node(struct pwrseq_pcie_m2_ctx *ctx,
+					struct device_node *parent)
+{
+	struct device *dev = ctx->dev;
+	struct device_node *np;
+	int ret;
+
+	ctx->ocs = devm_kzalloc(dev, sizeof(*ctx->ocs), GFP_KERNEL);
+	if (!ctx->ocs)
+		return -ENOMEM;
+
+	of_changeset_init(ctx->ocs);
+
+	np = of_changeset_create_node(ctx->ocs, parent, "bluetooth");
+	if (!np) {
+		dev_err(dev, "Failed to create bluetooth node\n");
+		ret = -ENODEV;
+		goto err_destroy_changeset;
+	}
+
+	ret = of_changeset_add_prop_string(ctx->ocs, np, "compatible", "qcom,wcn7850-bt");
+	if (ret) {
+		dev_err(dev, "Failed to add bluetooth compatible: %d\n", ret);
+		goto err_destroy_changeset;
+	}
+
+	ret = of_changeset_apply(ctx->ocs);
+	if (ret) {
+		dev_err(dev, "Failed to apply changeset: %d\n", ret);
+		goto err_destroy_changeset;
+	}
+
+	ret = device_add_of_node(&ctx->serdev->dev, np);
+	if (ret) {
+		dev_err(dev, "Failed to add OF node: %d\n", ret);
+		goto err_revert_changeset;
+	}
+
+	return 0;
+
+err_revert_changeset:
+	of_changeset_revert(ctx->ocs);
+err_destroy_changeset:
+	of_changeset_destroy(ctx->ocs);
+
+	return ret;
+}
+
+static int pwrseq_m2_pcie_notify(struct notifier_block *nb, unsigned long action,
+			      void *data)
+{
+	struct pwrseq_pcie_m2_ctx *ctx = container_of(nb, struct pwrseq_pcie_m2_ctx, nb);
+	struct pci_dev *pdev = to_pci_dev(data);
+	struct serdev_controller *serdev_ctrl;
+	struct device *dev = ctx->dev;
+	int ret;
+
+	/*
+	 * Check whether the PCI device is associated with this M.2 connector or
+	 * not, by comparing the OF node of the PCI device parent and the Port 0
+	 * (PCIe) remote node parent OF node.
+	 */
+	struct device_node *pci_parent __free(device_node) =
+			of_graph_get_remote_node(dev_of_node(ctx->dev), 0, 0);
+	if (!pci_parent || (pci_parent != pdev->dev.parent->of_node))
+		return NOTIFY_DONE;
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		/* Create serdev device for WCN7850 */
+		if (pdev->vendor == PCI_VENDOR_ID_QCOM && pdev->device == 0x1107) {
+			struct device_node *serdev_parent __free(device_node) =
+				of_graph_get_remote_node(dev_of_node(ctx->dev), 1, 1);
+			if (!serdev_parent)
+				return NOTIFY_DONE;
+
+			serdev_ctrl = of_find_serdev_controller_by_node(serdev_parent);
+			if (!serdev_ctrl)
+				return NOTIFY_DONE;
+
+			/*
+			 * Bail out if the device was already attached to this
+			 * controller.
+			 */
+			if (serdev_ctrl->serdev)
+				return NOTIFY_DONE;
+
+			ctx->serdev = serdev_device_alloc(serdev_ctrl);
+			if (!ctx->serdev)
+				return NOTIFY_BAD;
+
+			ret = pwrseq_m2_pcie_create_bt_node(ctx, serdev_parent);
+			if (ret) {
+				serdev_device_put(ctx->serdev);
+				return notifier_from_errno(ret);
+			}
+
+			ret = serdev_device_add(ctx->serdev);
+			if (ret) {
+				dev_err(dev, "Failed to add serdev for WCN7850: %d\n", ret);
+				of_changeset_revert(ctx->ocs);
+				of_changeset_destroy(ctx->ocs);
+				serdev_device_put(ctx->serdev);
+				return notifier_from_errno(ret);
+			}
+		}
+		break;
+	case BUS_NOTIFY_REMOVED_DEVICE:
+		/* Destroy serdev device for WCN7850 */
+		if (pdev->vendor == PCI_VENDOR_ID_QCOM && pdev->device == 0x1107) {
+			serdev_device_remove(ctx->serdev);
+			of_changeset_revert(ctx->ocs);
+			of_changeset_destroy(ctx->ocs);
+		}
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static bool pwrseq_pcie_m2_check_remote_node(struct device *dev, u8 port, u8 endpoint,
+					     const char *node)
+{
+	struct device_node *remote __free(device_node) =
+			of_graph_get_remote_node(dev_of_node(dev), port, endpoint);
+
+	if (remote && of_node_name_eq(remote, node))
+		return true;
+
+	return false;
+}
+
+/*
+ * If the connector exposes a non-discoverable bus like UART, the respective
+ * protocol device needs to be created manually with the help of the notifier
+ * of the discoverable bus like PCIe.
+ */
+static int pwrseq_pcie_m2_register_notifier(struct pwrseq_pcie_m2_ctx *ctx, struct device *dev)
+{
+	int ret;
+
+	/*
+	 * Register a PCI notifier for Key E connector that has PCIe as Port
+	 * 0/Endpoint 0 interface and Serial as Port 3/Endpoint 0 interface.
+	 */
+	if (pwrseq_pcie_m2_check_remote_node(dev, 3, 0, "serial")) {
+		if (pwrseq_pcie_m2_check_remote_node(dev, 0, 0, "pcie")) {
+			ctx->dev = dev;
+			ctx->nb.notifier_call = pwrseq_m2_pcie_notify;
+			ret = bus_register_notifier(&pci_bus_type, &ctx->nb);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to register notifier for serdev\n");
+		}
+	}
+
+	return 0;
+}
+
 static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -218,7 +386,7 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
 
 	ctx->num_vregs = ret;
 
-	ret = devm_add_action_or_reset(dev, pwrseq_pcie_m2_free_regulators, ctx);
+	ret = devm_add_action_or_reset(dev, pwrseq_pcie_m2_free_resources, ctx);
 	if (ret)
 		return ret;
 
@@ -233,7 +401,11 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(ctx->pwrseq),
 				     "Failed to register the power sequencer\n");
 
-	return 0;
+	/*
+	 * Register a notifier for creating protocol devices for
+	 * non-discoverable busses like UART.
+	 */
+	return pwrseq_pcie_m2_register_notifier(ctx, dev);
 }
 
 static const struct of_device_id pwrseq_pcie_m2_of_match[] = {

-- 
2.51.0
Re: [PATCH v6 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth
Posted by kernel test robot 2 weeks, 6 days ago
Hi Manivannan,

kernel test robot noticed the following build errors:

[auto build test ERROR on 559f264e403e4d58d56a17595c60a1de011c5e20]

url:    https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam-via-B4-Relay/serdev-Convert-to_serdev_-helpers-to-macros-and-use-container_of_const/20260317-123910
base:   559f264e403e4d58d56a17595c60a1de011c5e20
patch link:    https://lore.kernel.org/r/20260317-pci-m2-e-v6-9-9c898f108d3d%40oss.qualcomm.com
patch subject: [PATCH v6 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth
config: um-randconfig-002-20260318 (https://download.01.org/0day-ci/archive/20260318/202603180601.E8FFoQ4J-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 4abb927bacf37f18f6359a41639a6d1b3bffffb5)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260318/202603180601.E8FFoQ4J-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603180601.E8FFoQ4J-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/sched/rq-offsets.c:5:
   In file included from kernel/sched/sched.h:28:
   In file included from include/linux/cgroup_api.h:1:
   In file included from include/linux/cgroup.h:27:
   In file included from include/linux/kernel_stat.h:8:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/um/include/asm/hardirq.h:24:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:12:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:1209:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    1209 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
         |                                                   ~~~~~~~~~~ ^
   In file included from kernel/sched/rq-offsets.c:5:
   In file included from kernel/sched/sched.h:31:
   In file included from include/linux/cpufreq.h:17:
>> include/linux/of.h:1652:34: error: use of undeclared identifier 'OF_RECONFIG_ATTACH_NODE'; did you mean 'OF_RECONFIG_NO_CHANGE'?
    1652 |         return of_changeset_action(ocs, OF_RECONFIG_ATTACH_NODE, np, NULL);
         |                                         ^~~~~~~~~~~~~~~~~~~~~~~
         |                                         OF_RECONFIG_NO_CHANGE
   include/linux/of.h:1627:2: note: 'OF_RECONFIG_NO_CHANGE' declared here
    1627 |         OF_RECONFIG_NO_CHANGE = 0,
         |         ^
>> include/linux/of.h:1658:34: error: use of undeclared identifier 'OF_RECONFIG_DETACH_NODE'; did you mean 'OF_RECONFIG_NO_CHANGE'?
    1658 |         return of_changeset_action(ocs, OF_RECONFIG_DETACH_NODE, np, NULL);
         |                                         ^~~~~~~~~~~~~~~~~~~~~~~
         |                                         OF_RECONFIG_NO_CHANGE
   include/linux/of.h:1627:2: note: 'OF_RECONFIG_NO_CHANGE' declared here
    1627 |         OF_RECONFIG_NO_CHANGE = 0,
         |         ^
>> include/linux/of.h:1664:34: error: use of undeclared identifier 'OF_RECONFIG_ADD_PROPERTY'
    1664 |         return of_changeset_action(ocs, OF_RECONFIG_ADD_PROPERTY, np, prop);
         |                                         ^~~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/of.h:1670:34: error: use of undeclared identifier 'OF_RECONFIG_REMOVE_PROPERTY'
    1670 |         return of_changeset_action(ocs, OF_RECONFIG_REMOVE_PROPERTY, np, prop);
         |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/of.h:1676:34: error: use of undeclared identifier 'OF_RECONFIG_UPDATE_PROPERTY'
    1676 |         return of_changeset_action(ocs, OF_RECONFIG_UPDATE_PROPERTY, np, prop);
         |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning and 5 errors generated.
   make[3]: *** [scripts/Makefile.build:184: kernel/sched/rq-offsets.s] Error 1 shuffle=497447418
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1333: prepare0] Error 2 shuffle=497447418
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2 shuffle=497447418
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2 shuffle=497447418
   make: Target 'prepare' not remade because of errors.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for OF_DYNAMIC
   Depends on [n]: OF [=n]
   Selected by [y]:
   - POWER_SEQUENCING_PCIE_M2 [=y] && POWER_SEQUENCING [=y] && (PCI [=n] && OF [=n] || COMPILE_TEST [=y])


vim +1652 include/linux/of.h

2e8fff668dc14e Rob Herring       2023-03-29  1633  
201c910bd6898d Pantelis Antoniou 2014-07-04  1634  #ifdef CONFIG_OF_DYNAMIC
f6892d193fb9d6 Grant Likely      2014-11-21  1635  extern int of_reconfig_notifier_register(struct notifier_block *);
f6892d193fb9d6 Grant Likely      2014-11-21  1636  extern int of_reconfig_notifier_unregister(struct notifier_block *);
f5242e5a883bf1 Grant Likely      2014-11-24  1637  extern int of_reconfig_notify(unsigned long, struct of_reconfig_data *rd);
f5242e5a883bf1 Grant Likely      2014-11-24  1638  extern int of_reconfig_get_state_change(unsigned long action,
f5242e5a883bf1 Grant Likely      2014-11-24  1639  					struct of_reconfig_data *arg);
f6892d193fb9d6 Grant Likely      2014-11-21  1640  
201c910bd6898d Pantelis Antoniou 2014-07-04  1641  extern void of_changeset_init(struct of_changeset *ocs);
201c910bd6898d Pantelis Antoniou 2014-07-04  1642  extern void of_changeset_destroy(struct of_changeset *ocs);
201c910bd6898d Pantelis Antoniou 2014-07-04  1643  extern int of_changeset_apply(struct of_changeset *ocs);
201c910bd6898d Pantelis Antoniou 2014-07-04  1644  extern int of_changeset_revert(struct of_changeset *ocs);
201c910bd6898d Pantelis Antoniou 2014-07-04  1645  extern int of_changeset_action(struct of_changeset *ocs,
201c910bd6898d Pantelis Antoniou 2014-07-04  1646  		unsigned long action, struct device_node *np,
201c910bd6898d Pantelis Antoniou 2014-07-04  1647  		struct property *prop);
201c910bd6898d Pantelis Antoniou 2014-07-04  1648  
201c910bd6898d Pantelis Antoniou 2014-07-04  1649  static inline int of_changeset_attach_node(struct of_changeset *ocs,
201c910bd6898d Pantelis Antoniou 2014-07-04  1650  		struct device_node *np)
201c910bd6898d Pantelis Antoniou 2014-07-04  1651  {
201c910bd6898d Pantelis Antoniou 2014-07-04 @1652  	return of_changeset_action(ocs, OF_RECONFIG_ATTACH_NODE, np, NULL);
201c910bd6898d Pantelis Antoniou 2014-07-04  1653  }
201c910bd6898d Pantelis Antoniou 2014-07-04  1654  
201c910bd6898d Pantelis Antoniou 2014-07-04  1655  static inline int of_changeset_detach_node(struct of_changeset *ocs,
201c910bd6898d Pantelis Antoniou 2014-07-04  1656  		struct device_node *np)
201c910bd6898d Pantelis Antoniou 2014-07-04  1657  {
201c910bd6898d Pantelis Antoniou 2014-07-04 @1658  	return of_changeset_action(ocs, OF_RECONFIG_DETACH_NODE, np, NULL);
201c910bd6898d Pantelis Antoniou 2014-07-04  1659  }
201c910bd6898d Pantelis Antoniou 2014-07-04  1660  
201c910bd6898d Pantelis Antoniou 2014-07-04  1661  static inline int of_changeset_add_property(struct of_changeset *ocs,
201c910bd6898d Pantelis Antoniou 2014-07-04  1662  		struct device_node *np, struct property *prop)
201c910bd6898d Pantelis Antoniou 2014-07-04  1663  {
201c910bd6898d Pantelis Antoniou 2014-07-04 @1664  	return of_changeset_action(ocs, OF_RECONFIG_ADD_PROPERTY, np, prop);
201c910bd6898d Pantelis Antoniou 2014-07-04  1665  }
201c910bd6898d Pantelis Antoniou 2014-07-04  1666  
201c910bd6898d Pantelis Antoniou 2014-07-04  1667  static inline int of_changeset_remove_property(struct of_changeset *ocs,
201c910bd6898d Pantelis Antoniou 2014-07-04  1668  		struct device_node *np, struct property *prop)
201c910bd6898d Pantelis Antoniou 2014-07-04  1669  {
201c910bd6898d Pantelis Antoniou 2014-07-04 @1670  	return of_changeset_action(ocs, OF_RECONFIG_REMOVE_PROPERTY, np, prop);
201c910bd6898d Pantelis Antoniou 2014-07-04  1671  }
201c910bd6898d Pantelis Antoniou 2014-07-04  1672  
201c910bd6898d Pantelis Antoniou 2014-07-04  1673  static inline int of_changeset_update_property(struct of_changeset *ocs,
201c910bd6898d Pantelis Antoniou 2014-07-04  1674  		struct device_node *np, struct property *prop)
201c910bd6898d Pantelis Antoniou 2014-07-04  1675  {
201c910bd6898d Pantelis Antoniou 2014-07-04 @1676  	return of_changeset_action(ocs, OF_RECONFIG_UPDATE_PROPERTY, np, prop);
201c910bd6898d Pantelis Antoniou 2014-07-04  1677  }
b544fc2b8606d7 Lizhi Hou         2023-08-15  1678  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v6 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth
Posted by kernel test robot 2 weeks, 6 days ago
Hi Manivannan,

kernel test robot noticed the following build errors:

[auto build test ERROR on 559f264e403e4d58d56a17595c60a1de011c5e20]

url:    https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam-via-B4-Relay/serdev-Convert-to_serdev_-helpers-to-macros-and-use-container_of_const/20260317-123910
base:   559f264e403e4d58d56a17595c60a1de011c5e20
patch link:    https://lore.kernel.org/r/20260317-pci-m2-e-v6-9-9c898f108d3d%40oss.qualcomm.com
patch subject: [PATCH v6 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20260318/202603180609.ucspJefN-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260318/202603180609.ucspJefN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603180609.ucspJefN-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/power/sequencing/pwrseq-pcie-m2.c: In function 'pwrseq_pcie_m2_free_resources':
>> drivers/power/sequencing/pwrseq-pcie-m2.c:185:34: error: 'pci_bus_type' undeclared (first use in this function); did you mean 'pci_pcie_type'?
     185 |         bus_unregister_notifier(&pci_bus_type, &ctx->nb);
         |                                  ^~~~~~~~~~~~
         |                                  pci_pcie_type
   drivers/power/sequencing/pwrseq-pcie-m2.c:185:34: note: each undeclared identifier is reported only once for each function it appears in
   drivers/power/sequencing/pwrseq-pcie-m2.c: In function 'pwrseq_pcie_m2_register_notifier':
   drivers/power/sequencing/pwrseq-pcie-m2.c:340:54: error: 'pci_bus_type' undeclared (first use in this function); did you mean 'pci_pcie_type'?
     340 |                         ret = bus_register_notifier(&pci_bus_type, &ctx->nb);
         |                                                      ^~~~~~~~~~~~
         |                                                      pci_pcie_type


vim +185 drivers/power/sequencing/pwrseq-pcie-m2.c

   179	
   180	static void pwrseq_pcie_m2_free_resources(void *data)
   181	{
   182		struct pwrseq_pcie_m2_ctx *ctx = data;
   183	
   184		serdev_device_remove(ctx->serdev);
 > 185		bus_unregister_notifier(&pci_bus_type, &ctx->nb);
   186		of_changeset_revert(ctx->ocs);
   187		of_changeset_destroy(ctx->ocs);
   188		regulator_bulk_free(ctx->num_vregs, ctx->regs);
   189	}
   190	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v6 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth
Posted by Bartosz Golaszewski 2 weeks, 6 days ago
On Tue, 17 Mar 2026 05:29:59 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> For supporting bluetooth over the non-discoverable UART interface of
> WCN7850, create the serdev device after enumerating the PCIe interface.
> This is mandatory since the device ID is only known after the PCIe
> enumeration and the ID is used for creating the serdev device.
>
> Since by default there is no OF or ACPI node for the created serdev,
> create a dynamic OF 'bluetooth' node with the 'compatible' property and
> attach it to the serdev device. This will allow the serdev device to bind
> to the existing bluetooth driver.
>
> Tested-by: Hans de Goede <johannes.goede@oss.qualcomm.com> # ThinkPad T14s gen6 (arm64)
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/power/sequencing/Kconfig          |   3 +-
>  drivers/power/sequencing/pwrseq-pcie-m2.c | 178 +++++++++++++++++++++++++++++-
>  2 files changed, 177 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> index f5fff84566ba..55aeef125e6f 100644
> --- a/drivers/power/sequencing/Kconfig
> +++ b/drivers/power/sequencing/Kconfig
> @@ -37,7 +37,8 @@ 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 && OF) || COMPILE_TEST
> +	select OF_DYNAMIC
>  	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 3507cdcb1e7b..77357439ba81 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -12,9 +12,11 @@
>  #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>
> +#include <linux/serdev.h>
>  #include <linux/slab.h>
>
>  struct pwrseq_pcie_m2_pdata {
> @@ -30,6 +32,9 @@ struct pwrseq_pcie_m2_ctx {
>  	struct notifier_block nb;
>  	struct gpio_desc *w_disable1_gpio;
>  	struct gpio_desc *w_disable2_gpio;
> +	struct serdev_device *serdev;
> +	struct of_changeset *ocs;
> +	struct device *dev;
>  };
>
>  static int pwrseq_pcie_m2_vregs_enable(struct pwrseq_device *pwrseq)
> @@ -172,13 +177,176 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
>  	return PWRSEQ_NO_MATCH;
>  }
>
> -static void pwrseq_pcie_m2_free_regulators(void *data)
> +static void pwrseq_pcie_m2_free_resources(void *data)
>  {
>  	struct pwrseq_pcie_m2_ctx *ctx = data;
>
> +	serdev_device_remove(ctx->serdev);
> +	bus_unregister_notifier(&pci_bus_type, &ctx->nb);
> +	of_changeset_revert(ctx->ocs);
> +	of_changeset_destroy(ctx->ocs);
>  	regulator_bulk_free(ctx->num_vregs, ctx->regs);
>  }
>
> +static int pwrseq_m2_pcie_create_bt_node(struct pwrseq_pcie_m2_ctx *ctx,
> +					struct device_node *parent)
> +{
> +	struct device *dev = ctx->dev;
> +	struct device_node *np;
> +	int ret;
> +
> +	ctx->ocs = devm_kzalloc(dev, sizeof(*ctx->ocs), GFP_KERNEL);
> +	if (!ctx->ocs)
> +		return -ENOMEM;
> +
> +	of_changeset_init(ctx->ocs);
> +
> +	np = of_changeset_create_node(ctx->ocs, parent, "bluetooth");
> +	if (!np) {
> +		dev_err(dev, "Failed to create bluetooth node\n");
> +		ret = -ENODEV;
> +		goto err_destroy_changeset;
> +	}
> +
> +	ret = of_changeset_add_prop_string(ctx->ocs, np, "compatible", "qcom,wcn7850-bt");
> +	if (ret) {
> +		dev_err(dev, "Failed to add bluetooth compatible: %d\n", ret);
> +		goto err_destroy_changeset;
> +	}
> +
> +	ret = of_changeset_apply(ctx->ocs);
> +	if (ret) {
> +		dev_err(dev, "Failed to apply changeset: %d\n", ret);
> +		goto err_destroy_changeset;
> +	}
> +
> +	ret = device_add_of_node(&ctx->serdev->dev, np);
> +	if (ret) {
> +		dev_err(dev, "Failed to add OF node: %d\n", ret);
> +		goto err_revert_changeset;
> +	}
> +
> +	return 0;
> +
> +err_revert_changeset:
> +	of_changeset_revert(ctx->ocs);
> +err_destroy_changeset:
> +	of_changeset_destroy(ctx->ocs);
> +
> +	return ret;
> +}
> +
> +static int pwrseq_m2_pcie_notify(struct notifier_block *nb, unsigned long action,
> +			      void *data)
> +{
> +	struct pwrseq_pcie_m2_ctx *ctx = container_of(nb, struct pwrseq_pcie_m2_ctx, nb);
> +	struct pci_dev *pdev = to_pci_dev(data);
> +	struct serdev_controller *serdev_ctrl;
> +	struct device *dev = ctx->dev;
> +	int ret;
> +
> +	/*
> +	 * Check whether the PCI device is associated with this M.2 connector or
> +	 * not, by comparing the OF node of the PCI device parent and the Port 0
> +	 * (PCIe) remote node parent OF node.
> +	 */
> +	struct device_node *pci_parent __free(device_node) =
> +			of_graph_get_remote_node(dev_of_node(ctx->dev), 0, 0);
> +	if (!pci_parent || (pci_parent != pdev->dev.parent->of_node))
> +		return NOTIFY_DONE;
> +
> +	switch (action) {
> +	case BUS_NOTIFY_ADD_DEVICE:
> +		/* Create serdev device for WCN7850 */
> +		if (pdev->vendor == PCI_VENDOR_ID_QCOM && pdev->device == 0x1107) {
> +			struct device_node *serdev_parent __free(device_node) =
> +				of_graph_get_remote_node(dev_of_node(ctx->dev), 1, 1);
> +			if (!serdev_parent)
> +				return NOTIFY_DONE;
> +
> +			serdev_ctrl = of_find_serdev_controller_by_node(serdev_parent);
> +			if (!serdev_ctrl)
> +				return NOTIFY_DONE;
> +
> +			/*
> +			 * Bail out if the device was already attached to this
> +			 * controller.
> +			 */
> +			if (serdev_ctrl->serdev)
> +				return NOTIFY_DONE;
> +
> +			ctx->serdev = serdev_device_alloc(serdev_ctrl);
> +			if (!ctx->serdev)
> +				return NOTIFY_BAD;

If you bail out here, on driver unbind you'll call serdev_device_remove() which
uncoditionally dereferences the serdev pointer.

> +
> +			ret = pwrseq_m2_pcie_create_bt_node(ctx, serdev_parent);

If this doesn't succeed, ctx->ocs remains set to NULL (correct me if I'm wrong)
and you end up calling of_changeset_revert() which will unconditionally
dereference the of_changeset pointer in __of_changeset_entry_invert().

> +			if (ret) {
> +				serdev_device_put(ctx->serdev);
> +				return notifier_from_errno(ret);
> +			}
> +
> +			ret = serdev_device_add(ctx->serdev);
> +			if (ret) {
> +				dev_err(dev, "Failed to add serdev for WCN7850: %d\n", ret);
> +				of_changeset_revert(ctx->ocs);
> +				of_changeset_destroy(ctx->ocs);
> +				serdev_device_put(ctx->serdev);
> +				return notifier_from_errno(ret);
> +			}
> +		}
> +		break;
> +	case BUS_NOTIFY_REMOVED_DEVICE:
> +		/* Destroy serdev device for WCN7850 */
> +		if (pdev->vendor == PCI_VENDOR_ID_QCOM && pdev->device == 0x1107) {
> +			serdev_device_remove(ctx->serdev);
> +			of_changeset_revert(ctx->ocs);
> +			of_changeset_destroy(ctx->ocs);
> +		}
> +		break;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static bool pwrseq_pcie_m2_check_remote_node(struct device *dev, u8 port, u8 endpoint,
> +					     const char *node)
> +{
> +	struct device_node *remote __free(device_node) =
> +			of_graph_get_remote_node(dev_of_node(dev), port, endpoint);
> +
> +	if (remote && of_node_name_eq(remote, node))
> +		return true;
> +
> +	return false;
> +}
> +
> +/*
> + * If the connector exposes a non-discoverable bus like UART, the respective
> + * protocol device needs to be created manually with the help of the notifier
> + * of the discoverable bus like PCIe.
> + */
> +static int pwrseq_pcie_m2_register_notifier(struct pwrseq_pcie_m2_ctx *ctx, struct device *dev)
> +{
> +	int ret;
> +
> +	/*
> +	 * Register a PCI notifier for Key E connector that has PCIe as Port
> +	 * 0/Endpoint 0 interface and Serial as Port 3/Endpoint 0 interface.
> +	 */
> +	if (pwrseq_pcie_m2_check_remote_node(dev, 3, 0, "serial")) {
> +		if (pwrseq_pcie_m2_check_remote_node(dev, 0, 0, "pcie")) {
> +			ctx->dev = dev;
> +			ctx->nb.notifier_call = pwrseq_m2_pcie_notify;
> +			ret = bus_register_notifier(&pci_bus_type, &ctx->nb);
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "Failed to register notifier for serdev\n");
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -218,7 +386,7 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
>
>  	ctx->num_vregs = ret;
>
> -	ret = devm_add_action_or_reset(dev, pwrseq_pcie_m2_free_regulators, ctx);
> +	ret = devm_add_action_or_reset(dev, pwrseq_pcie_m2_free_resources, ctx);
>  	if (ret)
>  		return ret;
>
> @@ -233,7 +401,11 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(ctx->pwrseq),
>  				     "Failed to register the power sequencer\n");

If you bail out here, you will call bus_unregister_notifier() before your
registered it. Kernel docs are not clear on whether that's a bug but it still
looks fishy to me.

Am I not seeing something or is the error path broken? I'm doubting myself
here. :)

This is why I advised to split pwrseq_pcie_m2_free_resources() and only schedule
individual devres actions after their allocation succeeds.

As it is now, you're better off providing a remove() callback with NULL checks.

Bart

>
> -	return 0;
> +	/*
> +	 * Register a notifier for creating protocol devices for
> +	 * non-discoverable busses like UART.
> +	 */
> +	return pwrseq_pcie_m2_register_notifier(ctx, dev);
>  }
>
>  static const struct of_device_id pwrseq_pcie_m2_of_match[] = {
>
> --
> 2.51.0
>
>
>
Re: [PATCH v6 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth
Posted by Manivannan Sadhasivam 2 weeks, 5 days ago
On Tue, Mar 17, 2026 at 09:06:20AM -0400, Bartosz Golaszewski wrote:
> On Tue, 17 Mar 2026 05:29:59 +0100, Manivannan Sadhasivam via B4 Relay
> <devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> >
> > For supporting bluetooth over the non-discoverable UART interface of
> > WCN7850, create the serdev device after enumerating the PCIe interface.
> > This is mandatory since the device ID is only known after the PCIe
> > enumeration and the ID is used for creating the serdev device.
> >
> > Since by default there is no OF or ACPI node for the created serdev,
> > create a dynamic OF 'bluetooth' node with the 'compatible' property and
> > attach it to the serdev device. This will allow the serdev device to bind
> > to the existing bluetooth driver.
> >
> > Tested-by: Hans de Goede <johannes.goede@oss.qualcomm.com> # ThinkPad T14s gen6 (arm64)
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  drivers/power/sequencing/Kconfig          |   3 +-
> >  drivers/power/sequencing/pwrseq-pcie-m2.c | 178 +++++++++++++++++++++++++++++-
> >  2 files changed, 177 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> > index f5fff84566ba..55aeef125e6f 100644
> > --- a/drivers/power/sequencing/Kconfig
> > +++ b/drivers/power/sequencing/Kconfig
> > @@ -37,7 +37,8 @@ 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 && OF) || COMPILE_TEST
> > +	select OF_DYNAMIC
> >  	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 3507cdcb1e7b..77357439ba81 100644
> > --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> > +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> > @@ -12,9 +12,11 @@
> >  #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>
> > +#include <linux/serdev.h>
> >  #include <linux/slab.h>
> >
> >  struct pwrseq_pcie_m2_pdata {
> > @@ -30,6 +32,9 @@ struct pwrseq_pcie_m2_ctx {
> >  	struct notifier_block nb;
> >  	struct gpio_desc *w_disable1_gpio;
> >  	struct gpio_desc *w_disable2_gpio;
> > +	struct serdev_device *serdev;
> > +	struct of_changeset *ocs;
> > +	struct device *dev;
> >  };
> >
> >  static int pwrseq_pcie_m2_vregs_enable(struct pwrseq_device *pwrseq)
> > @@ -172,13 +177,176 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
> >  	return PWRSEQ_NO_MATCH;
> >  }
> >
> > -static void pwrseq_pcie_m2_free_regulators(void *data)
> > +static void pwrseq_pcie_m2_free_resources(void *data)
> >  {
> >  	struct pwrseq_pcie_m2_ctx *ctx = data;
> >
> > +	serdev_device_remove(ctx->serdev);
> > +	bus_unregister_notifier(&pci_bus_type, &ctx->nb);
> > +	of_changeset_revert(ctx->ocs);
> > +	of_changeset_destroy(ctx->ocs);
> >  	regulator_bulk_free(ctx->num_vregs, ctx->regs);
> >  }
> >
> > +static int pwrseq_m2_pcie_create_bt_node(struct pwrseq_pcie_m2_ctx *ctx,
> > +					struct device_node *parent)
> > +{
> > +	struct device *dev = ctx->dev;
> > +	struct device_node *np;
> > +	int ret;
> > +
> > +	ctx->ocs = devm_kzalloc(dev, sizeof(*ctx->ocs), GFP_KERNEL);
> > +	if (!ctx->ocs)
> > +		return -ENOMEM;
> > +
> > +	of_changeset_init(ctx->ocs);
> > +
> > +	np = of_changeset_create_node(ctx->ocs, parent, "bluetooth");
> > +	if (!np) {
> > +		dev_err(dev, "Failed to create bluetooth node\n");
> > +		ret = -ENODEV;
> > +		goto err_destroy_changeset;
> > +	}
> > +
> > +	ret = of_changeset_add_prop_string(ctx->ocs, np, "compatible", "qcom,wcn7850-bt");
> > +	if (ret) {
> > +		dev_err(dev, "Failed to add bluetooth compatible: %d\n", ret);
> > +		goto err_destroy_changeset;
> > +	}
> > +
> > +	ret = of_changeset_apply(ctx->ocs);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to apply changeset: %d\n", ret);
> > +		goto err_destroy_changeset;
> > +	}
> > +
> > +	ret = device_add_of_node(&ctx->serdev->dev, np);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to add OF node: %d\n", ret);
> > +		goto err_revert_changeset;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_revert_changeset:
> > +	of_changeset_revert(ctx->ocs);
> > +err_destroy_changeset:
> > +	of_changeset_destroy(ctx->ocs);
> > +
> > +	return ret;
> > +}
> > +
> > +static int pwrseq_m2_pcie_notify(struct notifier_block *nb, unsigned long action,
> > +			      void *data)
> > +{
> > +	struct pwrseq_pcie_m2_ctx *ctx = container_of(nb, struct pwrseq_pcie_m2_ctx, nb);
> > +	struct pci_dev *pdev = to_pci_dev(data);
> > +	struct serdev_controller *serdev_ctrl;
> > +	struct device *dev = ctx->dev;
> > +	int ret;
> > +
> > +	/*
> > +	 * Check whether the PCI device is associated with this M.2 connector or
> > +	 * not, by comparing the OF node of the PCI device parent and the Port 0
> > +	 * (PCIe) remote node parent OF node.
> > +	 */
> > +	struct device_node *pci_parent __free(device_node) =
> > +			of_graph_get_remote_node(dev_of_node(ctx->dev), 0, 0);
> > +	if (!pci_parent || (pci_parent != pdev->dev.parent->of_node))
> > +		return NOTIFY_DONE;
> > +
> > +	switch (action) {
> > +	case BUS_NOTIFY_ADD_DEVICE:
> > +		/* Create serdev device for WCN7850 */
> > +		if (pdev->vendor == PCI_VENDOR_ID_QCOM && pdev->device == 0x1107) {
> > +			struct device_node *serdev_parent __free(device_node) =
> > +				of_graph_get_remote_node(dev_of_node(ctx->dev), 1, 1);
> > +			if (!serdev_parent)
> > +				return NOTIFY_DONE;
> > +
> > +			serdev_ctrl = of_find_serdev_controller_by_node(serdev_parent);
> > +			if (!serdev_ctrl)
> > +				return NOTIFY_DONE;
> > +
> > +			/*
> > +			 * Bail out if the device was already attached to this
> > +			 * controller.
> > +			 */
> > +			if (serdev_ctrl->serdev)
> > +				return NOTIFY_DONE;
> > +
> > +			ctx->serdev = serdev_device_alloc(serdev_ctrl);
> > +			if (!ctx->serdev)
> > +				return NOTIFY_BAD;
> 
> If you bail out here, on driver unbind you'll call serdev_device_remove() which
> uncoditionally dereferences the serdev pointer.
> 
> > +
> > +			ret = pwrseq_m2_pcie_create_bt_node(ctx, serdev_parent);
> 
> If this doesn't succeed, ctx->ocs remains set to NULL (correct me if I'm wrong)
> and you end up calling of_changeset_revert() which will unconditionally
> dereference the of_changeset pointer in __of_changeset_entry_invert().
> 
> > +			if (ret) {
> > +				serdev_device_put(ctx->serdev);
> > +				return notifier_from_errno(ret);
> > +			}
> > +
> > +			ret = serdev_device_add(ctx->serdev);
> > +			if (ret) {
> > +				dev_err(dev, "Failed to add serdev for WCN7850: %d\n", ret);
> > +				of_changeset_revert(ctx->ocs);
> > +				of_changeset_destroy(ctx->ocs);
> > +				serdev_device_put(ctx->serdev);
> > +				return notifier_from_errno(ret);
> > +			}
> > +		}
> > +		break;
> > +	case BUS_NOTIFY_REMOVED_DEVICE:
> > +		/* Destroy serdev device for WCN7850 */
> > +		if (pdev->vendor == PCI_VENDOR_ID_QCOM && pdev->device == 0x1107) {
> > +			serdev_device_remove(ctx->serdev);
> > +			of_changeset_revert(ctx->ocs);
> > +			of_changeset_destroy(ctx->ocs);
> > +		}
> > +		break;
> > +	}
> > +
> > +	return NOTIFY_OK;
> > +}
> > +
> > +static bool pwrseq_pcie_m2_check_remote_node(struct device *dev, u8 port, u8 endpoint,
> > +					     const char *node)
> > +{
> > +	struct device_node *remote __free(device_node) =
> > +			of_graph_get_remote_node(dev_of_node(dev), port, endpoint);
> > +
> > +	if (remote && of_node_name_eq(remote, node))
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> > +/*
> > + * If the connector exposes a non-discoverable bus like UART, the respective
> > + * protocol device needs to be created manually with the help of the notifier
> > + * of the discoverable bus like PCIe.
> > + */
> > +static int pwrseq_pcie_m2_register_notifier(struct pwrseq_pcie_m2_ctx *ctx, struct device *dev)
> > +{
> > +	int ret;
> > +
> > +	/*
> > +	 * Register a PCI notifier for Key E connector that has PCIe as Port
> > +	 * 0/Endpoint 0 interface and Serial as Port 3/Endpoint 0 interface.
> > +	 */
> > +	if (pwrseq_pcie_m2_check_remote_node(dev, 3, 0, "serial")) {
> > +		if (pwrseq_pcie_m2_check_remote_node(dev, 0, 0, "pcie")) {
> > +			ctx->dev = dev;
> > +			ctx->nb.notifier_call = pwrseq_m2_pcie_notify;
> > +			ret = bus_register_notifier(&pci_bus_type, &ctx->nb);
> > +			if (ret)
> > +				return dev_err_probe(dev, ret,
> > +						     "Failed to register notifier for serdev\n");
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> > @@ -218,7 +386,7 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> >
> >  	ctx->num_vregs = ret;
> >
> > -	ret = devm_add_action_or_reset(dev, pwrseq_pcie_m2_free_regulators, ctx);
> > +	ret = devm_add_action_or_reset(dev, pwrseq_pcie_m2_free_resources, ctx);
> >  	if (ret)
> >  		return ret;
> >
> > @@ -233,7 +401,11 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> >  		return dev_err_probe(dev, PTR_ERR(ctx->pwrseq),
> >  				     "Failed to register the power sequencer\n");
> 
> If you bail out here, you will call bus_unregister_notifier() before your
> registered it. Kernel docs are not clear on whether that's a bug but it still
> looks fishy to me.
> 
> Am I not seeing something or is the error path broken? I'm doubting myself
> here. :)
> 

No, it was me who has gone bonkers with the error path. I was so delusional to
assume that these APIs handle NULL ptrs :/

> This is why I advised to split pwrseq_pcie_m2_free_resources() and only schedule
> individual devres actions after their allocation succeeds.
> 
> As it is now, you're better off providing a remove() callback with NULL checks.
> 

Makes sense. I'll fix them all in next version.

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v6 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth
Posted by Manivannan Sadhasivam 2 weeks, 6 days ago
On Tue, Mar 17, 2026 at 09:59:59AM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> For supporting bluetooth over the non-discoverable UART interface of
> WCN7850, create the serdev device after enumerating the PCIe interface.
> This is mandatory since the device ID is only known after the PCIe
> enumeration and the ID is used for creating the serdev device.
> 
> Since by default there is no OF or ACPI node for the created serdev,
> create a dynamic OF 'bluetooth' node with the 'compatible' property and
> attach it to the serdev device. This will allow the serdev device to bind
> to the existing bluetooth driver.
> 
> Tested-by: Hans de Goede <johannes.goede@oss.qualcomm.com> # ThinkPad T14s gen6 (arm64)
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/power/sequencing/Kconfig          |   3 +-
>  drivers/power/sequencing/pwrseq-pcie-m2.c | 178 +++++++++++++++++++++++++++++-
>  2 files changed, 177 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> index f5fff84566ba..55aeef125e6f 100644
> --- a/drivers/power/sequencing/Kconfig
> +++ b/drivers/power/sequencing/Kconfig
> @@ -37,7 +37,8 @@ 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 && OF) || COMPILE_TEST
> +	select OF_DYNAMIC
>  	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 3507cdcb1e7b..77357439ba81 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -12,9 +12,11 @@
>  #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>
> +#include <linux/serdev.h>
>  #include <linux/slab.h>
>  
>  struct pwrseq_pcie_m2_pdata {
> @@ -30,6 +32,9 @@ struct pwrseq_pcie_m2_ctx {
>  	struct notifier_block nb;
>  	struct gpio_desc *w_disable1_gpio;
>  	struct gpio_desc *w_disable2_gpio;
> +	struct serdev_device *serdev;
> +	struct of_changeset *ocs;
> +	struct device *dev;
>  };
>  
>  static int pwrseq_pcie_m2_vregs_enable(struct pwrseq_device *pwrseq)
> @@ -172,13 +177,176 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
>  	return PWRSEQ_NO_MATCH;
>  }
>  
> -static void pwrseq_pcie_m2_free_regulators(void *data)
> +static void pwrseq_pcie_m2_free_resources(void *data)
>  {
>  	struct pwrseq_pcie_m2_ctx *ctx = data;
>  
> +	serdev_device_remove(ctx->serdev);
> +	bus_unregister_notifier(&pci_bus_type, &ctx->nb);
> +	of_changeset_revert(ctx->ocs);
> +	of_changeset_destroy(ctx->ocs);
>  	regulator_bulk_free(ctx->num_vregs, ctx->regs);
>  }
>  
> +static int pwrseq_m2_pcie_create_bt_node(struct pwrseq_pcie_m2_ctx *ctx,
> +					struct device_node *parent)
> +{
> +	struct device *dev = ctx->dev;
> +	struct device_node *np;
> +	int ret;
> +
> +	ctx->ocs = devm_kzalloc(dev, sizeof(*ctx->ocs), GFP_KERNEL);
> +	if (!ctx->ocs)
> +		return -ENOMEM;
> +
> +	of_changeset_init(ctx->ocs);
> +
> +	np = of_changeset_create_node(ctx->ocs, parent, "bluetooth");
> +	if (!np) {
> +		dev_err(dev, "Failed to create bluetooth node\n");
> +		ret = -ENODEV;
> +		goto err_destroy_changeset;
> +	}
> +
> +	ret = of_changeset_add_prop_string(ctx->ocs, np, "compatible", "qcom,wcn7850-bt");
> +	if (ret) {
> +		dev_err(dev, "Failed to add bluetooth compatible: %d\n", ret);
> +		goto err_destroy_changeset;
> +	}
> +
> +	ret = of_changeset_apply(ctx->ocs);
> +	if (ret) {
> +		dev_err(dev, "Failed to apply changeset: %d\n", ret);
> +		goto err_destroy_changeset;
> +	}
> +
> +	ret = device_add_of_node(&ctx->serdev->dev, np);
> +	if (ret) {
> +		dev_err(dev, "Failed to add OF node: %d\n", ret);
> +		goto err_revert_changeset;
> +	}
> +
> +	return 0;
> +
> +err_revert_changeset:
> +	of_changeset_revert(ctx->ocs);
> +err_destroy_changeset:
> +	of_changeset_destroy(ctx->ocs);
> +
> +	return ret;
> +}
> +
> +static int pwrseq_m2_pcie_notify(struct notifier_block *nb, unsigned long action,
> +			      void *data)
> +{
> +	struct pwrseq_pcie_m2_ctx *ctx = container_of(nb, struct pwrseq_pcie_m2_ctx, nb);
> +	struct pci_dev *pdev = to_pci_dev(data);
> +	struct serdev_controller *serdev_ctrl;
> +	struct device *dev = ctx->dev;
> +	int ret;
> +
> +	/*
> +	 * Check whether the PCI device is associated with this M.2 connector or
> +	 * not, by comparing the OF node of the PCI device parent and the Port 0
> +	 * (PCIe) remote node parent OF node.
> +	 */
> +	struct device_node *pci_parent __free(device_node) =
> +			of_graph_get_remote_node(dev_of_node(ctx->dev), 0, 0);
> +	if (!pci_parent || (pci_parent != pdev->dev.parent->of_node))
> +		return NOTIFY_DONE;
> +
> +	switch (action) {
> +	case BUS_NOTIFY_ADD_DEVICE:
> +		/* Create serdev device for WCN7850 */
> +		if (pdev->vendor == PCI_VENDOR_ID_QCOM && pdev->device == 0x1107) {
> +			struct device_node *serdev_parent __free(device_node) =
> +				of_graph_get_remote_node(dev_of_node(ctx->dev), 1, 1);

Typo. This should be:

	of_graph_get_remote_node(dev_of_node(ctx->dev), 3, 0);

Bartosz, could you please fix it while applying?

- Mani

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