[PATCH 0/6] ASoC: qcom: Fix confusing cleanup.h

[PATCH 0/6] ASoC: qcom: Fix confusing cleanup.h

[Krzysztof Kozlowski]

Please, please stop ending cleanup.h patches for very simple code like:

  foo = kzalloc();
  kfree(foo);
  return;

... *if you do not intend to read cleanup.h*. These changes are making
simple code not necessarily simpler. But worse, if you do not read
cleanup.h then you introduce actually undesired, error-prone and wrong
style of having constructors with redundant values (= NULL).

This is actually worse code.

If you do not agree in declaration-in-place-of-use (fair!), then do not
use cleanup.h. If you want to use cleanup.h, then please read cleanup.h
before.

This is second mixup I see recently around Qualcomm files.

Best regards,
Krzysztof

---
Krzysztof Kozlowski (6):
      ASoC: qcom: q6prm: Fix confusing cleanup.h syntax
      ASoC: qcom: q6asm: Fix confusing cleanup.h syntax
      ASoC: qcom: q6apm: Fix confusing cleanup.h syntax
      ASoC: qcom: q6afe: Fix confusing cleanup.h syntax
      ASoC: qcom: audioreach: Fix confusing cleanup.h syntax
      ASoC: qcom: Minor readability improve with new lines

 sound/soc/qcom/qdsp6/audioreach.c | 28 +++++++++++++++++-----------
 sound/soc/qcom/qdsp6/q6adm.c      |  2 ++
 sound/soc/qcom/qdsp6/q6afe.c      | 10 ++++++----
 sound/soc/qcom/qdsp6/q6apm.c      |  9 ++++++---
 sound/soc/qcom/qdsp6/q6asm.c      | 19 +++++++++++++++----
 sound/soc/qcom/qdsp6/q6prm.c      | 16 ++++++++--------
 6 files changed, 54 insertions(+), 30 deletions(-)
---
base-commit: 92fd6e84175befa1775e5c0ab682938eca27c0b2
change-id: 20251129-asoc-wrong-cleanup-h-can-people-stop-sending-this-without-reading-docs-ebe8abad6e55

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

[PATCH 1/6] ASoC: qcom: q6prm: Fix confusing cleanup.h syntax

[Krzysztof Kozlowski]

Commit de8e95773c48 ("ASoc: qcom: q6prm: Use automatic cleanup of
kfree()") did not make the code simpler but more complicated.  Already
simple code of allocation and free, without any error paths, got now
declaration with one constructor followed by another allocation, which
is in contrary to explicit coding rules guiding cleanup.h:

"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."

Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

---

Can we pleas stop sending these cleanup.h patches when people do not
bother to read cleanup.h?
---
 sound/soc/qcom/qdsp6/q6prm.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6prm.c b/sound/soc/qcom/qdsp6/q6prm.c
index 0b8fad0bc832..2544c4519b26 100644
--- a/sound/soc/qcom/qdsp6/q6prm.c
+++ b/sound/soc/qcom/qdsp6/q6prm.c
@@ -62,7 +62,6 @@ static int q6prm_set_hw_core_req(struct device *dev, uint32_t hw_block_id, bool
 	struct prm_cmd_request_hw_core *req;
 	gpr_device_t *gdev = prm->gdev;
 	uint32_t opcode, rsp_opcode;
-	struct gpr_pkt *pkt __free(kfree) = NULL;
 
 	if (enable) {
 		opcode = PRM_CMD_REQUEST_HW_RSC;
@@ -72,7 +71,8 @@ static int q6prm_set_hw_core_req(struct device *dev, uint32_t hw_block_id, bool
 		rsp_opcode = PRM_CMD_RSP_RELEASE_HW_RSC;
 	}
 
-	pkt = audioreach_alloc_cmd_pkt(sizeof(*req), opcode, 0, gdev->svc.id, GPR_PRM_MODULE_IID);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_cmd_pkt(sizeof(*req), opcode, 0, gdev->svc.id, GPR_PRM_MODULE_IID);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -111,10 +111,10 @@ static int q6prm_request_lpass_clock(struct device *dev, int clk_id, int clk_att
 	struct apm_module_param_data *param_data;
 	struct prm_cmd_request_rsc *req;
 	gpr_device_t *gdev = prm->gdev;
-	struct gpr_pkt *pkt __free(kfree) = NULL;
 
-	pkt = audioreach_alloc_cmd_pkt(sizeof(*req), PRM_CMD_REQUEST_HW_RSC, 0, gdev->svc.id,
-				       GPR_PRM_MODULE_IID);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_cmd_pkt(sizeof(*req), PRM_CMD_REQUEST_HW_RSC, 0,
+					 gdev->svc.id, GPR_PRM_MODULE_IID);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -143,10 +143,10 @@ static int q6prm_release_lpass_clock(struct device *dev, int clk_id, int clk_att
 	struct apm_module_param_data *param_data;
 	struct prm_cmd_release_rsc *rel;
 	gpr_device_t *gdev = prm->gdev;
-	struct gpr_pkt *pkt __free(kfree) = NULL;
 
-	pkt = audioreach_alloc_cmd_pkt(sizeof(*rel), PRM_CMD_RELEASE_HW_RSC, 0, gdev->svc.id,
-				       GPR_PRM_MODULE_IID);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_cmd_pkt(sizeof(*rel), PRM_CMD_RELEASE_HW_RSC, 0,
+					 gdev->svc.id, GPR_PRM_MODULE_IID);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 

-- 
2.48.1

[PATCH 2/6] ASoC: qcom: q6asm: Fix confusing cleanup.h syntax

[Krzysztof Kozlowski]

Commit 6e00112d31c8 ("ASoc: qcom: q6asm: Use automatic cleanup of
kfree()") did not make the code simpler but more complicated.  Already
simple code of allocation and free, without any error paths, got now
declaration with one constructor followed by another allocation, which
is in contrary to explicit coding rules guiding cleanup.h:

"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."

Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/q6asm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6asm.c b/sound/soc/qcom/qdsp6/q6asm.c
index e7295b7b2461..890a1f786627 100644
--- a/sound/soc/qcom/qdsp6/q6asm.c
+++ b/sound/soc/qcom/qdsp6/q6asm.c
@@ -335,7 +335,6 @@ static int __q6asm_memory_unmap(struct audio_client *ac,
 	struct q6asm *a = dev_get_drvdata(ac->dev->parent);
 	struct apr_pkt *pkt;
 	int rc, pkt_size;
-	void *p __free(kfree) = NULL;
 
 	if (ac->port[dir].mem_map_handle == 0) {
 		dev_err(ac->dev, "invalid mem handle\n");
@@ -343,7 +342,7 @@ static int __q6asm_memory_unmap(struct audio_client *ac,
 	}
 
 	pkt_size = APR_HDR_SIZE + sizeof(*mem_unmap);
-	p = kzalloc(pkt_size, GFP_KERNEL);
+	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 
@@ -428,7 +427,6 @@ static int __q6asm_memory_map_regions(struct audio_client *ac, int dir,
 	struct audio_port_data *port = NULL;
 	struct audio_buffer *ab = NULL;
 	struct apr_pkt *pkt;
-	void *p __free(kfree) = NULL;
 	unsigned long flags;
 	uint32_t num_regions, buf_sz;
 	int i, pkt_size;
@@ -447,7 +445,7 @@ static int __q6asm_memory_map_regions(struct audio_client *ac, int dir,
 	pkt_size = APR_HDR_SIZE + sizeof(*cmd) +
 		   (sizeof(*mregions) * num_regions);
 
-	p = kzalloc(pkt_size, GFP_KERNEL);
+	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 

-- 
2.48.1

[PATCH 3/6] ASoC: qcom: q6apm: Fix confusing cleanup.h syntax

[Krzysztof Kozlowski]

Commit 89cf2223ee7b ("ASoc: qcom: q6apm: Use automatic cleanup of
kfree()") did not make the code simpler but more complicated.  Already
simple code of allocation and free, without any error paths, got now
declaration with one constructor followed by another allocation, which
is in contrary to explicit coding rules guiding cleanup.h:

"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."

Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/q6apm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
index 94cc6376a367..4e5ad04ece50 100644
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -259,7 +259,6 @@ int q6apm_unmap_memory_regions(struct q6apm_graph *graph, unsigned int dir)
 {
 	struct apm_cmd_shared_mem_unmap_regions *cmd;
 	struct audioreach_graph_data *data;
-	struct gpr_pkt *pkt __free(kfree) = NULL;
 	int rc;
 
 	if (dir == SNDRV_PCM_STREAM_PLAYBACK)
@@ -270,8 +269,9 @@ int q6apm_unmap_memory_regions(struct q6apm_graph *graph, unsigned int dir)
 	if (!data->mem_map_handle)
 		return 0;
 
-	pkt = audioreach_alloc_apm_pkt(sizeof(*cmd), APM_CMD_SHARED_MEM_UNMAP_REGIONS, dir,
-				     graph->port->id);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_pkt(sizeof(*cmd), APM_CMD_SHARED_MEM_UNMAP_REGIONS,
+					 dir, graph->port->id);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 

-- 
2.48.1

[PATCH 4/6] ASoC: qcom: q6afe: Fix confusing cleanup.h syntax

[Krzysztof Kozlowski]

Commit 55094e55ae36 ("ASoc: qcom: q6afe: Use automatic cleanup of
kfree()") did not make the code simpler but more complicated.  Already
simple code of allocation and free, without any error paths, got now
declaration with one constructor followed by another allocation, which
is in contrary to explicit coding rules guiding cleanup.h:

"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."

Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/q6afe.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
index 980851a12976..c9fdd53492e0 100644
--- a/sound/soc/qcom/qdsp6/q6afe.c
+++ b/sound/soc/qcom/qdsp6/q6afe.c
@@ -1277,7 +1277,6 @@ int q6afe_port_stop(struct q6afe_port *port)
 	int port_id = port->id;
 	int ret = 0;
 	int index, pkt_size;
-	void *p __free(kfree) = NULL;
 
 	index = port->token;
 	if (index < 0 || index >= AFE_PORT_MAX) {
@@ -1286,7 +1285,7 @@ int q6afe_port_stop(struct q6afe_port *port)
 	}
 
 	pkt_size = APR_HDR_SIZE + sizeof(*stop);
-	p = kzalloc(pkt_size, GFP_KERNEL);
+	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 
@@ -1667,7 +1666,6 @@ int q6afe_port_start(struct q6afe_port *port)
 	int ret, param_id = port->cfg_type;
 	struct apr_pkt *pkt;
 	int pkt_size;
-	void *p __free(kfree) = NULL;
 
 	ret  = q6afe_port_set_param_v2(port, &port->port_cfg, param_id,
 				       AFE_MODULE_AUDIO_DEV_INTERFACE,
@@ -1690,7 +1688,7 @@ int q6afe_port_start(struct q6afe_port *port)
 	}
 
 	pkt_size = APR_HDR_SIZE + sizeof(*start);
-	p = kzalloc(pkt_size, GFP_KERNEL);
+	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 

-- 
2.48.1

[PATCH 5/6] ASoC: qcom: audioreach: Fix confusing cleanup.h syntax

[Krzysztof Kozlowski]

Commit 88a5f8e628ef ("ASoc: qcom: audioreach: Use automatic cleanup of
kfree()") did not make the code simpler but more complicated.  Already
simple code of allocation and free, without any error paths, got now
declaration with one constructor followed by another allocation, which
is in contrary to explicit coding rules guiding cleanup.h:

"Given that the "__free(...) = NULL" pattern for variables defined at
the top of the function poses this potential interdependency problem the
recommendation is to always define and assign variables in one statement
and not group variable definitions at the top of the function when
__free() is used."

Code does not have a bug, but is less readable and uses discouraged
coding practice, so fix that by moving declaration to the place of
assignment.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/audioreach.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
index ded49124581b..329d916779f0 100644
--- a/sound/soc/qcom/qdsp6/audioreach.c
+++ b/sound/soc/qcom/qdsp6/audioreach.c
@@ -730,15 +730,15 @@ int audioreach_send_u32_param(struct q6apm_graph *graph, struct audioreach_modul
 			      uint32_t param_id, uint32_t param_val)
 {
 	struct apm_module_param_data *param_data;
-	struct gpr_pkt *pkt __free(kfree) = NULL;
 	uint32_t *param;
 	int payload_size = sizeof(uint32_t) + APM_MODULE_PARAM_DATA_SIZE;
-	void *p = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
-	if (IS_ERR(p))
+	void *p;
+
+	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
+	if (IS_ERR(pkt))
 		return -ENOMEM;
 
-	pkt = p;
-	p = p + GPR_HDR_SIZE + APM_CMD_HDR_SIZE;
+	p = (void *)pkt + GPR_HDR_SIZE + APM_CMD_HDR_SIZE;
 
 	param_data = p;
 	param_data->module_instance_id = module->instance_id;
@@ -1043,7 +1043,6 @@ static int audioreach_pcm_set_media_format(struct q6apm_graph *graph,
 	struct apm_pcm_module_media_fmt_cmd *cfg;
 	struct apm_module_param_data *param_data;
 	int payload_size;
-	struct gpr_pkt *pkt __free(kfree) = NULL;
 
 	if (num_channels > 4) {
 		dev_err(graph->dev, "Error: Invalid channels (%d)!\n", num_channels);
@@ -1052,7 +1051,8 @@ static int audioreach_pcm_set_media_format(struct q6apm_graph *graph,
 
 	payload_size = APM_PCM_MODULE_FMT_CMD_PSIZE(num_channels);
 
-	pkt = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -1090,7 +1090,6 @@ static int audioreach_shmem_set_media_format(struct q6apm_graph *graph,
 	struct payload_media_fmt_pcm *cfg;
 	struct media_format *header;
 	int rc, payload_size;
-	struct gpr_pkt *pkt __free(kfree) = NULL;
 	void *p;
 
 	if (num_channels > 4) {
@@ -1100,8 +1099,9 @@ static int audioreach_shmem_set_media_format(struct q6apm_graph *graph,
 
 	payload_size = APM_SHMEM_FMT_CFG_PSIZE(num_channels) + APM_MODULE_PARAM_DATA_SIZE;
 
-	pkt = audioreach_alloc_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0,
-				     graph->port->id, module->instance_id);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0,
+					 graph->port->id, module->instance_id);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 

-- 
2.48.1

[PATCH 6/6] ASoC: qcom: Minor readability improve with new lines

[Krzysztof Kozlowski]

Variables with automatic cleanup are special because they do not follow
standard rules of declaration at top of function (see cleanup.h), but on
the other hand we always expect line break between top-function
declarations and first instructions.

Don't pretend automatic cleanup variables are part of top-level
declaration to improve readability when variable is followed by nun-NULL
check.  No functional impact, only style.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/audioreach.c |  8 +++++++-
 sound/soc/qcom/qdsp6/q6adm.c      |  2 ++
 sound/soc/qcom/qdsp6/q6afe.c      |  4 ++++
 sound/soc/qcom/qdsp6/q6apm.c      |  3 +++
 sound/soc/qcom/qdsp6/q6asm.c      | 13 +++++++++++++
 5 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
index 329d916779f0..f3fa0a5b4095 100644
--- a/sound/soc/qcom/qdsp6/audioreach.c
+++ b/sound/soc/qcom/qdsp6/audioreach.c
@@ -617,6 +617,7 @@ static int audioreach_display_port_set_media_format(struct q6apm_graph *graph,
 	int fs_sz = APM_FS_CFG_PSIZE;
 	int size = ic_sz + ep_sz + fs_sz;
 	void *p;
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
@@ -675,6 +676,7 @@ static int audioreach_codec_dma_set_media_format(struct q6apm_graph *graph,
 	int pm_sz = APM_HW_EP_PMODE_CFG_PSIZE;
 	int size = ic_sz + ep_sz + fs_sz + pm_sz;
 	void *p;
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
@@ -788,6 +790,7 @@ static int audioreach_set_module_config(struct q6apm_graph *graph,
 {
 	int size = le32_to_cpu(module->data->size);
 	void *p;
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
@@ -810,6 +813,7 @@ static int audioreach_mfc_set_media_format(struct q6apm_graph *graph,
 				APM_MODULE_PARAM_DATA_SIZE;
 	int i;
 	void *p;
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
@@ -922,13 +926,13 @@ int audioreach_compr_set_param(struct q6apm_graph *graph, struct audioreach_modu
 	void *p;
 	int iid = q6apm_graph_get_rx_shmem_module_iid(graph);
 	int payload_size = sizeof(struct apm_sh_module_media_fmt_cmd);
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_cmd_pkt(payload_size,
 					DATA_CMD_WR_SH_MEM_EP_MEDIA_FORMAT,
 					0, graph->port->id, iid);
 	if (IS_ERR(pkt))
 		return -ENOMEM;
 
-
 	p = (void *)pkt + GPR_HDR_SIZE;
 	header = p;
 	rc = audioreach_set_compr_media_format(header, p, mcfg);
@@ -952,6 +956,7 @@ static int audioreach_i2s_set_media_format(struct q6apm_graph *graph,
 	int fs_sz = APM_FS_CFG_PSIZE;
 	int size = ic_sz + ep_sz + fs_sz;
 	void *p;
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
@@ -1013,6 +1018,7 @@ static int audioreach_logging_set_media_format(struct q6apm_graph *graph,
 	struct data_logging_config *cfg;
 	int size = sizeof(*cfg) + APM_MODULE_PARAM_DATA_SIZE;
 	void *p;
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
diff --git a/sound/soc/qcom/qdsp6/q6adm.c b/sound/soc/qcom/qdsp6/q6adm.c
index 0b8d06ec8b26..bbe986293ec3 100644
--- a/sound/soc/qcom/qdsp6/q6adm.c
+++ b/sound/soc/qcom/qdsp6/q6adm.c
@@ -331,6 +331,7 @@ static int q6adm_device_open(struct q6adm *adm, struct q6copp *copp,
 	int afe_port = q6afe_get_port_id(port_id);
 	struct apr_pkt *pkt;
 	int ret, pkt_size = APR_HDR_SIZE + sizeof(*open);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -466,6 +467,7 @@ int q6adm_matrix_map(struct device *dev, int path,
 	struct q6copp *copp;
 	int pkt_size = (APR_HDR_SIZE + sizeof(*route) +  sizeof(*node) +
 		    (sizeof(uint32_t) * payload_map.num_copps));
+
 	void *matrix_map __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!matrix_map)
 		return -ENOMEM;
diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
index c9fdd53492e0..0cf6aebe498c 100644
--- a/sound/soc/qcom/qdsp6/q6afe.c
+++ b/sound/soc/qcom/qdsp6/q6afe.c
@@ -1077,6 +1077,7 @@ static int q6afe_set_param(struct q6afe *afe, struct q6afe_port *port,
 	struct apr_pkt *pkt;
 	int ret, pkt_size = APR_HDR_SIZE + sizeof(*param) + sizeof(*pdata) + psize;
 	void *pl;
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1128,6 +1129,7 @@ static int q6afe_port_set_param_v2(struct q6afe_port *port, void *data,
 	u16 port_id = port->id;
 	int ret, pkt_size = APR_HDR_SIZE + sizeof(*param) + sizeof(*pdata) + psize;
 	void *pl;
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1832,6 +1834,7 @@ int q6afe_unvote_lpass_core_hw(struct device *dev, uint32_t hw_block_id,
 	struct apr_pkt *pkt;
 	int ret = 0;
 	int pkt_size = APR_HDR_SIZE + sizeof(*vote_cfg);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1866,6 +1869,7 @@ int q6afe_vote_lpass_core_hw(struct device *dev, uint32_t hw_block_id,
 	struct apr_pkt *pkt;
 	int ret = 0;
 	int pkt_size = APR_HDR_SIZE + sizeof(*vote_cfg);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
index 4e5ad04ece50..e30f8648ae15 100644
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -100,6 +100,7 @@ static int audioreach_graph_mgmt_cmd(struct audioreach_graph *graph, uint32_t op
 	struct audioreach_sub_graph *sg;
 	struct q6apm *apm = graph->apm;
 	int i = 0, payload_size = APM_GRAPH_MGMT_PSIZE(mgmt_cmd, num_sub_graphs);
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(payload_size, opcode, 0);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
@@ -409,6 +410,7 @@ int q6apm_write_async(struct q6apm_graph *graph, uint32_t len, uint32_t msw_ts,
 	struct apm_data_cmd_wr_sh_mem_ep_data_buffer_v2 *write_buffer;
 	struct audio_buffer *ab;
 	int iid = q6apm_graph_get_rx_shmem_module_iid(graph);
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_pkt(sizeof(*write_buffer),
 					DATA_CMD_WR_SH_MEM_EP_DATA_BUFFER_V2,
 					graph->rx_data.dsp_buf | (len << APM_WRITE_TOKEN_LEN_SHIFT),
@@ -446,6 +448,7 @@ int q6apm_read(struct q6apm_graph *graph)
 	struct audioreach_graph_data *port;
 	struct audio_buffer *ab;
 	int iid = q6apm_graph_get_tx_shmem_module_iid(graph);
+
 	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_pkt(sizeof(*read_buffer),
 					DATA_CMD_RD_SH_MEM_EP_DATA_BUFFER_V2,
 					graph->tx_data.dsp_buf, graph->port->id, iid);
diff --git a/sound/soc/qcom/qdsp6/q6asm.c b/sound/soc/qcom/qdsp6/q6asm.c
index 890a1f786627..420176f80ffe 100644
--- a/sound/soc/qcom/qdsp6/q6asm.c
+++ b/sound/soc/qcom/qdsp6/q6asm.c
@@ -928,6 +928,7 @@ int q6asm_open_write(struct audio_client *ac, uint32_t stream_id,
 	struct asm_stream_cmd_open_write_v3 *open;
 	struct apr_pkt *pkt;
 	int rc, pkt_size = APR_HDR_SIZE + sizeof(*open);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1005,6 +1006,7 @@ static int __q6asm_run(struct audio_client *ac, uint32_t stream_id,
 	struct asm_session_cmd_run_v2 *run;
 	struct apr_pkt *pkt;
 	int rc, pkt_size = APR_HDR_SIZE + sizeof(*run);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_ATOMIC);
 	if (!p)
 		return -ENOMEM;
@@ -1087,6 +1089,7 @@ int q6asm_media_format_block_multi_ch_pcm(struct audio_client *ac,
 	struct apr_pkt *pkt;
 	u8 *channel_mapping;
 	int pkt_size = APR_HDR_SIZE + sizeof(*fmt);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1125,6 +1128,7 @@ int q6asm_stream_media_format_block_flac(struct audio_client *ac,
 	struct asm_flac_fmt_blk_v2 *fmt;
 	struct apr_pkt *pkt;
 	int pkt_size = APR_HDR_SIZE + sizeof(*fmt);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1156,6 +1160,7 @@ int q6asm_stream_media_format_block_wma_v9(struct audio_client *ac,
 	struct asm_wmastdv9_fmt_blk_v2 *fmt;
 	struct apr_pkt *pkt;
 	int pkt_size = APR_HDR_SIZE + sizeof(*fmt);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1188,6 +1193,7 @@ int q6asm_stream_media_format_block_wma_v10(struct audio_client *ac,
 	struct asm_wmaprov10_fmt_blk_v2 *fmt;
 	struct apr_pkt *pkt;
 	int pkt_size = APR_HDR_SIZE + sizeof(*fmt);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1221,6 +1227,7 @@ int q6asm_stream_media_format_block_alac(struct audio_client *ac,
 	struct asm_alac_fmt_blk_v2 *fmt;
 	struct apr_pkt *pkt;
 	int pkt_size = APR_HDR_SIZE + sizeof(*fmt);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1257,6 +1264,7 @@ int q6asm_stream_media_format_block_ape(struct audio_client *ac,
 	struct asm_ape_fmt_blk_v2 *fmt;
 	struct apr_pkt *pkt;
 	int pkt_size = APR_HDR_SIZE + sizeof(*fmt);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1291,6 +1299,7 @@ static int q6asm_stream_remove_silence(struct audio_client *ac, uint32_t stream_
 	uint32_t *samples;
 	struct apr_pkt *pkt;
 	int rc, pkt_size = APR_HDR_SIZE + sizeof(uint32_t);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_ATOMIC);
 	if (!p)
 		return -ENOMEM;
@@ -1349,6 +1358,7 @@ int q6asm_enc_cfg_blk_pcm_format_support(struct audio_client *ac,
 	u8 *channel_mapping;
 	u32 frames_per_buf = 0;
 	int pkt_size = APR_HDR_SIZE + sizeof(*enc_cfg);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1395,6 +1405,7 @@ int q6asm_read(struct audio_client *ac, uint32_t stream_id)
 	unsigned long flags;
 	int pkt_size = APR_HDR_SIZE + sizeof(*read);
 	int rc = 0;
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_ATOMIC);
 	if (!p)
 		return -ENOMEM;
@@ -1437,6 +1448,7 @@ static int __q6asm_open_read(struct audio_client *ac, uint32_t stream_id,
 	struct asm_stream_cmd_open_read_v3 *open;
 	struct apr_pkt *pkt;
 	int pkt_size = APR_HDR_SIZE + sizeof(*open);
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
@@ -1507,6 +1519,7 @@ int q6asm_write_async(struct audio_client *ac, uint32_t stream_id, uint32_t len,
 	struct apr_pkt *pkt;
 	int pkt_size = APR_HDR_SIZE + sizeof(*write);
 	int rc = 0;
+
 	void *p __free(kfree) = kzalloc(pkt_size, GFP_ATOMIC);
 	if (!p)
 		return -ENOMEM;

-- 
2.48.1

Re: [PATCH 0/6] ASoC: qcom: Fix confusing cleanup.h

[Srinivas Kandagatla]

On 11/29/25 1:17 PM, Krzysztof Kozlowski wrote:
> Please, please stop ending cleanup.h patches for very simple code like:
> 
>   foo = kzalloc();
>   kfree(foo);
>   return;
> 
> ... *if you do not intend to read cleanup.h*. These changes are making
> simple code not necessarily simpler. But worse, if you do not read
> cleanup.h then you introduce actually undesired, error-prone and wrong
> style of having constructors with redundant values (= NULL).
> 
> This is actually worse code.
> 
> If you do not agree in declaration-in-place-of-use (fair!), then do not
> use cleanup.h. If you want to use cleanup.h, then please read cleanup.h
> before.
> 
> This is second mixup I see recently around Qualcomm files.
> 
> Best regards,
> Krzysztof
> 
> ---
> Krzysztof Kozlowski (6):
>       ASoC: qcom: q6prm: Fix confusing cleanup.h syntax
>       ASoC: qcom: q6asm: Fix confusing cleanup.h syntax
>       ASoC: qcom: q6apm: Fix confusing cleanup.h syntax
>       ASoC: qcom: q6afe: Fix confusing cleanup.h syntax
>       ASoC: qcom: audioreach: Fix confusing cleanup.h syntax
>       ASoC: qcom: Minor readability improve with new lines
> 

Thanks Krzysztof for fixing the confusing parts, I do agree with you.

there are alteast 100+ such instances in just sound/*


Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>

--srini

>  sound/soc/qcom/qdsp6/audioreach.c | 28 +++++++++++++++++-----------
>  sound/soc/qcom/qdsp6/q6adm.c      |  2 ++
>  sound/soc/qcom/qdsp6/q6afe.c      | 10 ++++++----
>  sound/soc/qcom/qdsp6/q6apm.c      |  9 ++++++---
>  sound/soc/qcom/qdsp6/q6asm.c      | 19 +++++++++++++++----
>  sound/soc/qcom/qdsp6/q6prm.c      | 16 ++++++++--------
>  6 files changed, 54 insertions(+), 30 deletions(-)
> ---
> base-commit: 92fd6e84175befa1775e5c0ab682938eca27c0b2
> change-id: 20251129-asoc-wrong-cleanup-h-can-people-stop-sending-this-without-reading-docs-ebe8abad6e55
> 
> Best regards,

Re: [PATCH 0/6] ASoC: qcom: Fix confusing cleanup.h

[Krzysztof Kozlowski]

On 01/12/2025 16:12, Srinivas Kandagatla wrote:

>> Krzysztof
>>
>> ---
>> Krzysztof Kozlowski (6):
>>       ASoC: qcom: q6prm: Fix confusing cleanup.h syntax
>>       ASoC: qcom: q6asm: Fix confusing cleanup.h syntax
>>       ASoC: qcom: q6apm: Fix confusing cleanup.h syntax
>>       ASoC: qcom: q6afe: Fix confusing cleanup.h syntax
>>       ASoC: qcom: audioreach: Fix confusing cleanup.h syntax
>>       ASoC: qcom: Minor readability improve with new lines
>>
> 
> Thanks Krzysztof for fixing the confusing parts, I do agree with you.
> 
> there are alteast 100+ such instances in just sound/*
> 

More patches then! I have a long flight to Tokyo for that. :)

Best regards,
Krzysztof

Re: [PATCH 0/6] ASoC: qcom: Fix confusing cleanup.h

[Mark Brown]

On Sat, 29 Nov 2025 14:17:52 +0100, Krzysztof Kozlowski wrote:
> Please, please stop ending cleanup.h patches for very simple code like:
> 
>   foo = kzalloc();
>   kfree(foo);
>   return;
> 
> ... *if you do not intend to read cleanup.h*. These changes are making
> simple code not necessarily simpler. But worse, if you do not read
> cleanup.h then you introduce actually undesired, error-prone and wrong
> style of having constructors with redundant values (= NULL).
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/6] ASoC: qcom: q6prm: Fix confusing cleanup.h syntax
      commit: bd9e7182e36169cd7e1ea3b25b5c82b1c5698e64
[2/6] ASoC: qcom: q6asm: Fix confusing cleanup.h syntax
      commit: c862dc9019f517893eb83096076d7eed4ecbb372
[3/6] ASoC: qcom: q6apm: Fix confusing cleanup.h syntax
      commit: 310e6f95eedaae04990072078adbb38beb149811
[4/6] ASoC: qcom: q6afe: Fix confusing cleanup.h syntax
      commit: 3c84bfa47ff29ec0c202cb139d365421c6778d65
[5/6] ASoC: qcom: audioreach: Fix confusing cleanup.h syntax
      commit: 0e6071d656fb284e003a45ce158831d4d12aac5a
[6/6] ASoC: qcom: Minor readability improve with new lines
      commit: 0bb160c92ad400c692984763996b758458adea17

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark