Skip to contentSkip to conversation

FAILED: patch "[PATCH] i2c: qcom-cci: fix device_node refcount leak in" failed to apply to 5.15-stable tree

PATCH New [email protected] 2 participants 5 messages Sep 22

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.15.y
git checkout FETCH_HEAD
git cherry-pick -x 7362a1553eb09a8cdf8be7e509bd5309a8342486
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026092212-fruit-deploy-01d6@gregkh' --subject-prefix 'PATCH 5.15.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 7362a1553eb09a8cdf8be7e509bd5309a8342486 Mon Sep 17 00:00:00 2001
From: Liu Zhenlong <[email protected]>
Date: Wed, 19 Aug 2026 01:57:50 +0800
Subject: [PATCH] i2c: qcom-cci: fix device_node refcount leak in
cci_probe()/cci_remove()

The of_node_put() matching of_node_get() runs after i2c_del_adapter(), whose trailing memset() zeroes adap->dev and thus adap->dev.of_node, making the put a no-op and leaking the node on every adapter removal and error cleanup.

Use a devm action: the pointer is captured at registration, out of reach of that memset(), and devres runs the put once on probe failure and detach, replacing the three manual of_node_put() calls. The setup loop uses the scoped iterator form so the child node is released automatically if devm_add_action_or_reset() fails mid-loop.

Fixes 02a4a69667a2 ("i2c: qcom-cci: don't put a device tree node before i2c_add_adapter()")
Assisted-by Claude:claude-opus-5
Cc <[email protected]> # v5.17+
drivers/i2c/busses/i2c-qcom-cci.c +11 -9
index 25b6e4e9e3fa..873e901a23d7 100644
@@ -497,10 +497,14 @@ static const struct dev_pm_ops qcom_cci_pm = {
 	SET_RUNTIME_PM_OPS(cci_suspend_runtime, cci_resume_runtime, NULL)
 };
 
+static void cci_put_of_node(void *data)
+{
+	of_node_put(data);
+}
+
 static int cci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *child;
 	struct resource *r;
 	struct cci *cci;
 	int ret, i;
@@ -516,7 +520,7 @@ static int cci_probe(struct platform_device *pdev)
 	if (!cci->data)
 		return -ENOENT;
 
-	for_each_available_child_of_node(dev->of_node, child) {
+	for_each_available_child_of_node_scoped(dev->of_node, child) {
 		struct cci_master *master;
 		u32 idx;
 
@@ -537,6 +541,9 @@ static int cci_probe(struct platform_device *pdev)
 		master->adap.algo = &cci_algo;
 		master->adap.dev.parent = dev;
 		master->adap.dev.of_node = of_node_get(child);
+		ret = devm_add_action_or_reset(dev, cci_put_of_node, child);
+		if (ret)
+			return ret;
 		master->master = idx;
 		master->cci = cci;
 
@@ -606,10 +613,8 @@ static int cci_probe(struct platform_device *pdev)
 			continue;
 
 		ret = i2c_add_adapter(&cci->master[i].adap);
-		if (ret < 0) {
-			of_node_put(cci->master[i].adap.dev.of_node);
+		if (ret < 0)
 			goto error_i2c;
-		}
 	}
 
 	return 0;
@@ -617,10 +622,8 @@ static int cci_probe(struct platform_device *pdev)
 error_i2c:
 
 	for (--i ; i >= 0; i--) {
-		if (cci->master[i].cci) {
+		if (cci->master[i].cci)
 			i2c_del_adapter(&cci->master[i].adap);
-			of_node_put(cci->master[i].adap.dev.of_node);
-		}
 	}
 disable_clocks:
 	cci_disable_clocks(cci);
@@ -636,7 +639,6 @@ static void cci_remove(struct platform_device *pdev)
 	for (i = 0; i < cci->data->num_masters; i++) {
 		if (cci->master[i].cci) {
 			i2c_del_adapter(&cci->master[i].adap);
-			of_node_put(cci->master[i].adap.dev.of_node);
 			cci_halt(cci, i);
 		}
 	}
maintainer
#1 lore raw

PATCH 1/4 of: Introduce for_each_*_child_of_node_scoped() to automate of_node_put() handling

From: Jonathan Cameron <[email protected]>

[ Upstream commit 34af4554fb0ce164e2c4876683619eb1e23848d4 ]

To avoid issues with out of order cleanup, or ambiguity about when the auto freed data is first instantiated, do it within the for loop definition.

The disadvantage is that the struct device_node *child variable creation is not immediately obvious where this is used.
However, in many cases, if there is another definition of struct device_node *child; the compiler / static analysers will notify us that it is unused, or uninitialized.

Note that, in the vast majority of cases, the _available_ form should be used and as code is converted to these scoped handers, we should confirm that any cases that do not check for available have a good reason not to.

Stable-dep-of: 7362a1553eb0 ("i2c: qcom-cci: fix device_node refcount leak in cci_probe()/cci_remove()")

---
 include/linux/of.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)
include/linux/of.h +13 -0
index 3c840c4879956..97352f3afed50 100644
@@ -1291,10 +1291,23 @@ static inline int of_property_read_s32(const struct device_node *np,
 #define for_each_child_of_node(parent, child) \
 	for (child = of_get_next_child(parent, NULL); child != NULL; \
 	     child = of_get_next_child(parent, child))
+
+#define for_each_child_of_node_scoped(parent, child) \
+	for (struct device_node *child __free(device_node) =		\
+	     of_get_next_child(parent, NULL);				\
+	     child != NULL;						\
+	     child = of_get_next_child(parent, child))
+
 #define for_each_available_child_of_node(parent, child) \
 	for (child = of_get_next_available_child(parent, NULL); child != NULL; \
 	     child = of_get_next_available_child(parent, child))
 
+#define for_each_available_child_of_node_scoped(parent, child) \
+	for (struct device_node *child __free(device_node) =		\
+	     of_get_next_available_child(parent, NULL);			\
+	     child != NULL;						\
+	     child = of_get_next_available_child(parent, child))
+
 #define for_each_of_cpu_node(cpu) \
 	for (cpu = of_get_next_cpu_node(NULL); cpu != NULL; \
 	     cpu = of_get_next_cpu_node(cpu))
Signature · 1 line
2.53.0
maintainer
#2 lore raw

PATCH 2/4 i2c: qcom-cci: Stop complaining about DT set clock rate

From: Bryan O'Donoghue <[email protected]>

[ Upstream commit 8284750a182937bf805f703311501730f02eb40e ]

It is common practice in the downstream and upstream CCI dt to set CCI clock rates to 19.2 MHz. It appears to be fairly common for initial code to set the CCI clock rate to 37.5 MHz.

Applying the widely used CCI clock rates from downstream ought not to cause warning messages in the upstream kernel where our general policy is to usually copy downstream hardware clock rates across the range of Qualcomm drivers.

Drop the warning it is pervasive across CAMSS users but doesn't add any information or warrant any changes to the DT to align the DT clock rate to the bootloader clock rate.

Stable-dep-of: 7362a1553eb0 ("i2c: qcom-cci: fix device_node refcount leak in cci_probe()/cci_remove()")

---
 drivers/i2c/busses/i2c-qcom-cci.c | 8 --------
 1 file changed, 8 deletions(-)
drivers/i2c/busses/i2c-qcom-cci.c +0 -8
index eae21899e0f3c..657840f5639c8 100644
@@ -602,14 +602,6 @@ static int cci_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (cci_clk_rate != cci->data->cci_clk_rate) {
-		/* cci clock set by the bootloader or via assigned clock rate
-		 * in DT.
-		 */
-		dev_warn(dev, "Found %lu cci clk rate while %lu was expected\n",
-			 cci_clk_rate, cci->data->cci_clk_rate);
-	}
-
 	ret = cci_enable_clocks(cci);
 	if (ret < 0)
 		return ret;
Signature · 1 line
2.53.0
maintainer
#3 lore raw

PATCH 3/4 i2c: qcom-cci: Remove the unused variable cci_clk_rate

In reply to Sasha Levin

From: Jiapeng Chong <[email protected]>

[ Upstream commit b88c79699d72caa947ecaae85839b3563662ccce ]

Variable cci_clk_rate is not effectively used, so delete it.

drivers/i2c/busses/i2c-qcom-cci.c:526:16: warning: variable ‘cci_clk_rate’ set but not used.

Stable-dep-of: 7362a1553eb0 ("i2c: qcom-cci: fix device_node refcount leak in cci_probe()/cci_remove()")

---
 drivers/i2c/busses/i2c-qcom-cci.c | 9 ---------
 1 file changed, 9 deletions(-)
drivers/i2c/busses/i2c-qcom-cci.c +0 -9
index 657840f5639c8..6a4d8a546fa75 100644
@@ -523,7 +523,6 @@ static const struct dev_pm_ops qcom_cci_pm = {
 static int cci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	unsigned long cci_clk_rate = 0;
 	struct device_node *child;
 	struct resource *r;
 	struct cci *cci;
@@ -594,14 +593,6 @@ static int cci_probe(struct platform_device *pdev)
 	}
 	cci->nclocks = ret;
 
-	/* Retrieve CCI clock rate */
-	for (i = 0; i < cci->nclocks; i++) {
-		if (!strcmp(cci->clocks[i].id, "cci")) {
-			cci_clk_rate = clk_get_rate(cci->clocks[i].clk);
-			break;
-		}
-	}
-
 	ret = cci_enable_clocks(cci);
 	if (ret < 0)
 		return ret;
Signature · 1 line
2.53.0
maintainer
#4 lore raw

PATCH 4/4 i2c: qcom-cci: fix device_node refcount leak in cci_probe()/cci_remove()

In reply to Sasha Levin

From: Liu Zhenlong <[email protected]>

[ Upstream commit 7362a1553eb09a8cdf8be7e509bd5309a8342486 ]

The of_node_put() matching of_node_get() runs after i2c_del_adapter(), whose trailing memset() zeroes adap->dev and thus adap->dev.of_node, making the put a no-op and leaking the node on every adapter removal and error cleanup.

Use a devm action: the pointer is captured at registration, out of reach of that memset(), and devres runs the put once on probe failure and detach, replacing the three manual of_node_put() calls. The setup loop uses the scoped iterator form so the child node is released automatically if devm_add_action_or_reset() fails mid-loop.

Fixes 02a4a69667a2 ("i2c: qcom-cci: don't put a device tree node before i2c_add_adapter()")
Assisted-by Claude:claude-opus-5
Cc <[email protected]> # v5.17+
[ Replaced the scoped iterator with the existing iterator and a function-scoped cleanup variable for GNU89 compatibility. ]
---
 drivers/i2c/busses/i2c-qcom-cci.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)
drivers/i2c/busses/i2c-qcom-cci.c +11 -8
index 6a4d8a546fa75..f3af3a14d2b57 100644
@@ -520,10 +520,15 @@ static const struct dev_pm_ops qcom_cci_pm = {
 	SET_RUNTIME_PM_OPS(cci_suspend_runtime, cci_resume_runtime, NULL)
 };
 
+static void cci_put_of_node(void *data)
+{
+	of_node_put(data);
+}
+
 static int cci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *child;
+	struct device_node *child __free(device_node) = NULL;
 	struct resource *r;
 	struct cci *cci;
 	int ret, i;
@@ -558,6 +563,9 @@ static int cci_probe(struct platform_device *pdev)
 		cci->master[idx].adap.algo = &cci_algo;
 		cci->master[idx].adap.dev.parent = dev;
 		cci->master[idx].adap.dev.of_node = of_node_get(child);
+		ret = devm_add_action_or_reset(dev, cci_put_of_node, child);
+		if (ret)
+			return ret;
 		cci->master[idx].master = idx;
 		cci->master[idx].cci = cci;
 
@@ -631,10 +639,8 @@ static int cci_probe(struct platform_device *pdev)
 			continue;
 
 		ret = i2c_add_adapter(&cci->master[i].adap);
-		if (ret < 0) {
-			of_node_put(cci->master[i].adap.dev.of_node);
+		if (ret < 0)
 			goto error_i2c;
-		}
 	}
 
 	return 0;
@@ -644,10 +650,8 @@ static int cci_probe(struct platform_device *pdev)
 	pm_runtime_dont_use_autosuspend(dev);
 
 	for (--i ; i >= 0; i--) {
-		if (cci->master[i].cci) {
+		if (cci->master[i].cci)
 			i2c_del_adapter(&cci->master[i].adap);
-			of_node_put(cci->master[i].adap.dev.of_node);
-		}
 	}
 error:
 	disable_irq(cci->irq);
@@ -665,7 +669,6 @@ static int cci_remove(struct platform_device *pdev)
 	for (i = 0; i < cci->data->num_masters; i++) {
 		if (cci->master[i].cci) {
 			i2c_del_adapter(&cci->master[i].adap);
-			of_node_put(cci->master[i].adap.dev.of_node);
 			cci_halt(cci, i);
 		}
 	}
Signature · 1 line
2.53.0