[PATCH 6/6] ASoC: qcom: Use helper function for_each_child_of_node_scoped()

Ai Chao posted 6 patches 7 months ago
There is a newer version of this series
[PATCH 6/6] ASoC: qcom: Use helper function for_each_child_of_node_scoped()
Posted by Ai Chao 7 months ago
The for_each_child_of_node_scoped() helper provides a scope-based
clean-up functionality to put the device_node automatically, and
as such, there is no need to call of_node_put() directly.

Thus, use this helper to simplify the code.

Signed-off-by: Ai Chao <aichao@kylinos.cn>
---
 sound/soc/qcom/lpass-cpu.c       | 3 +--
 sound/soc/qcom/qdsp6/q6afe-dai.c | 3 +--
 sound/soc/qcom/qdsp6/q6asm-dai.c | 4 +---
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 242bc16da36d..62f49fe46273 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -1046,7 +1046,6 @@ static unsigned int of_lpass_cpu_parse_sd_lines(struct device *dev,
 static void of_lpass_cpu_parse_dai_data(struct device *dev,
 					struct lpass_data *data)
 {
-	struct device_node *node;
 	int ret, i, id;
 
 	/* Allow all channels by default for backwards compatibility */
@@ -1056,7 +1055,7 @@ static void of_lpass_cpu_parse_dai_data(struct device *dev,
 		data->mi2s_capture_sd_mode[id] = LPAIF_I2SCTL_MODE_8CH;
 	}
 
-	for_each_child_of_node(dev->of_node, node) {
+	for_each_child_of_node_scoped(dev->of_node, node) {
 		ret = of_property_read_u32(node, "reg", &id);
 		if (ret || id < 0) {
 			dev_err(dev, "valid dai id not found: %d\n", ret);
diff --git a/sound/soc/qcom/qdsp6/q6afe-dai.c b/sound/soc/qcom/qdsp6/q6afe-dai.c
index 7d9628cda875..64735f2adf8f 100644
--- a/sound/soc/qcom/qdsp6/q6afe-dai.c
+++ b/sound/soc/qcom/qdsp6/q6afe-dai.c
@@ -962,10 +962,9 @@ static const struct snd_soc_component_driver q6afe_dai_component = {
 static void of_q6afe_parse_dai_data(struct device *dev,
 				    struct q6afe_dai_data *data)
 {
-	struct device_node *node;
 	int ret;
 
-	for_each_child_of_node(dev->of_node, node) {
+	for_each_child_of_node_scoped(dev->of_node, node) {
 		unsigned int lines[Q6AFE_MAX_MI2S_LINES];
 		struct q6afe_dai_priv_data *priv;
 		int id, i, num_lines;
diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c b/sound/soc/qcom/qdsp6/q6asm-dai.c
index a400c9a31fea..d7680dd3a3bb 100644
--- a/sound/soc/qcom/qdsp6/q6asm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
@@ -1236,10 +1236,8 @@ static int of_q6asm_parse_dai_data(struct device *dev,
 {
 	struct snd_soc_dai_driver *dai_drv;
 	struct snd_soc_pcm_stream empty_stream;
-	struct device_node *node;
 	int ret, id, dir, idx = 0;
 
-
 	pdata->num_dais = of_get_child_count(dev->of_node);
 	if (!pdata->num_dais) {
 		dev_err(dev, "No dais found in DT\n");
@@ -1253,7 +1251,7 @@ static int of_q6asm_parse_dai_data(struct device *dev,
 
 	memset(&empty_stream, 0, sizeof(empty_stream));
 
-	for_each_child_of_node(dev->of_node, node) {
+	for_each_child_of_node_scoped(dev->of_node, node) {
 		ret = of_property_read_u32(node, "reg", &id);
 		if (ret || id >= MAX_SESSIONS || id < 0) {
 			dev_err(dev, "valid dai id not found:%d\n", ret);
-- 
2.47.1
Re: [PATCH 6/6] ASoC: qcom: Use helper function for_each_child_of_node_scoped()
Posted by Krzysztof Kozlowski 7 months ago
On 20/05/2025 11:11, Ai Chao wrote:
> The for_each_child_of_node_scoped() helper provides a scope-based
> clean-up functionality to put the device_node automatically, and
> as such, there is no need to call of_node_put() directly.


I do not see any of_node_put() there, so I don't understand what is the
benefit of this and how this commit msg explains reason behind the change.

Best regards,
Krzysztof
Re: [PATCH 6/6] ASoC: qcom: Use helper function for_each_child_of_node_scoped()
Posted by Ai Chao 7 months ago
Hi Krzysztof :

     Thanks for your help.

     The for_each_child_of_node() function is used to iterate over all 
child nodes of a device tree node. During each iteration, it retrieves a 
pointer to the child node via of_get_next_child() and automatically 
increments the node's reference count (of_node_get()). Each call to 
of_get_next_child() increases the reference count (refcount) of the 
returned child node, ensuring that the node is not freed while in use.

     The of_node_put() function is used to decrement the node's 
reference count. When the reference count drops to zero, the kernel 
releases the memory occupied by the node.for_each_child_of_node() 
increments the child node's reference count in each iteration but does 
not decrement it automatically. If of_node_put() is not called manually, 
the reference count will never reach zero, resulting in a memory leak of 
the node.

On 2025/5/20 17:19, Krzysztof Kozlowski wrote:
> On 20/05/2025 11:11, Ai Chao wrote:
>> The for_each_child_of_node_scoped() helper provides a scope-based
>> clean-up functionality to put the device_node automatically, and
>> as such, there is no need to call of_node_put() directly.
>
> I do not see any of_node_put() there, so I don't understand what is the
> benefit of this and how this commit msg explains reason behind the change.
>
> Best regards,
> Krzysztof
Re: [PATCH 6/6] ASoC: qcom: Use helper function for_each_child_of_node_scoped()
Posted by Krzysztof Kozlowski 7 months ago
On 21/05/2025 03:58, Ai Chao wrote:
> Hi Krzysztof :
> 
>      Thanks for your help.
> 
>      The for_each_child_of_node() function is used to iterate over all 
> child nodes of a device tree node. During each iteration, it retrieves a 
> pointer to the child node via of_get_next_child() and automatically 
> increments the node's reference count (of_node_get()). Each call to 
> of_get_next_child() increases the reference count (refcount) of the 
> returned child node, ensuring that the node is not freed while in use.

Don't lecture us on the API, we know it.

> 
>      The of_node_put() function is used to decrement the node's 
> reference count. When the reference count drops to zero, the kernel 
> releases the memory occupied by the node.for_each_child_of_node() 
> increments the child node's reference count in each iteration but does 
> not decrement it automatically. If of_node_put() is not called manually, 
> the reference count will never reach zero, resulting in a memory leak of 
> the node.

Except top posting, you did not answer the comment at all. Explaining
all this in view of existing code means you do not understand the code.

Best regards,
Krzysztof