Re: [PATCH 2/4] firmware: qcom: scm: use __free(qcom_tzmem) to simplify cleanup

[Mukesh Ojha]

On Wed, Jul 01, 2026 at 03:38:56PM +0200, Bartosz Golaszewski wrote:
> Use the __free(qcom_tzmem) cleanup attribute (together with no_free_ptr()
> whenever ownership is transferred) to replace open-coded
> qcom_tzmem_free() calls and their associated goto labels.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 48 +++++++++++++++++-----------------------
>  1 file changed, 20 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 6b601a4b89dbf04322730aa22fc9590d180a383c..1684220617748e4890ecb45a1787a6798dafd5cc 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -640,10 +640,9 @@ static int qcom_scm_pas_prep_and_init_image(struct qcom_scm_pas_context *ctx,
>  {
>  	struct qcom_scm_res res;
>  	phys_addr_t mdata_phys;
> -	void *mdata_buf;
>  	int ret;
>  
> -	mdata_buf = qcom_tzmem_alloc(__scm->mempool, size, GFP_KERNEL);
> +	void *mdata_buf __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool, size, GFP_KERNEL);
>  	if (!mdata_buf)
>  		return -ENOMEM;
>  
> @@ -652,11 +651,10 @@ static int qcom_scm_pas_prep_and_init_image(struct qcom_scm_pas_context *ctx,
>  
>  	ret = __qcom_scm_pas_init_image(ctx->pas_id, mdata_phys, &res);
>  	if (ret < 0)
> -		qcom_tzmem_free(mdata_buf);
> -	else
> -		ctx->ptr = mdata_buf;
> +		return ret;
>  
> -	return ret ? : res.result[0];
> +	ctx->ptr = no_free_ptr(mdata_buf);
> +	return res.result[0];
>  }
>  
>  /**
> @@ -791,10 +789,11 @@ static void *__qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt_tzm,
>  		.owner = ARM_SMCCC_OWNER_SIP,
>  	};
>  	struct qcom_scm_res res;
> -	void *output_rt_tzm;
>  	int ret;
>  
> -	output_rt_tzm = qcom_tzmem_alloc(__scm->mempool, *output_rt_size, GFP_KERNEL);
> +	void *output_rt_tzm __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool,
> +								   *output_rt_size,
> +								   GFP_KERNEL);
>  	if (!output_rt_tzm)
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -814,20 +813,17 @@ static void *__qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt_tzm,
>  	 * be of unresonable size.
>  	 */
>  	ret = qcom_scm_call(__scm->dev, &desc, &res);
> -	if (!ret && res.result[2] > SZ_1G) {
> -		ret = -E2BIG;
> -		goto free_output_rt;
> -	}
> +	if (!ret && res.result[2] > SZ_1G)
> +		return ERR_PTR(-E2BIG);
>  
>  	*output_rt_size = res.result[2];
>  	if (ret && res.result[1] == RSCTABLE_BUFFER_NOT_SUFFICIENT)
> -		ret = -EOVERFLOW;
> +		return ERR_PTR(-EOVERFLOW);
>  
> -free_output_rt:
>  	if (ret)
> -		qcom_tzmem_free(output_rt_tzm);
> +		return ERR_PTR(ret);
>  
> -	return ret ? ERR_PTR(ret) : output_rt_tzm;
> +	return no_free_ptr(output_rt_tzm);
>  }
>  
>  /**
> @@ -878,8 +874,6 @@ struct resource_table *qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *c
>  {
>  	struct resource_table empty_rsc = {};
>  	size_t size = SZ_16K;
> -	void *output_rt_tzm;
> -	void *input_rt_tzm;
>  	void *tbl_ptr;
>  	int ret;
>  
> @@ -901,7 +895,9 @@ struct resource_table *qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *c
>  		input_rt_size = sizeof(empty_rsc);
>  	}
>  
> -	input_rt_tzm = qcom_tzmem_alloc(__scm->mempool, input_rt_size, GFP_KERNEL);
> +	void *input_rt_tzm __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool,
> +								  input_rt_size,
> +								  GFP_KERNEL);
>  	if (!input_rt_tzm) {
>  		ret = -ENOMEM;
>  		goto disable_scm_bw;
> @@ -909,8 +905,9 @@ struct resource_table *qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *c
>  
>  	memcpy(input_rt_tzm, input_rt, input_rt_size);
>  
> -	output_rt_tzm = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt_tzm,
> -						     input_rt_size, &size);
> +	void *output_rt_tzm __free(qcom_tzmem) =
> +		__qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt_tzm,
> +					     input_rt_size, &size);
>  	if (PTR_ERR(output_rt_tzm) == -EOVERFLOW)
>  		/* Try again with the size requested by the TZ */
>  		output_rt_tzm = __qcom_scm_pas_get_rsc_table(ctx->pas_id,
> @@ -919,21 +916,16 @@ struct resource_table *qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *c
>  							     &size);
>  	if (IS_ERR(output_rt_tzm)) {
>  		ret = PTR_ERR(output_rt_tzm);
> -		goto free_input_rt;
> +		goto disable_scm_bw;
>  	}
>  
>  	tbl_ptr = kmemdup(output_rt_tzm, size, GFP_KERNEL);
>  	if (!tbl_ptr) {
> -		qcom_tzmem_free(output_rt_tzm);
>  		ret = -ENOMEM;
> -		goto free_input_rt;
> +		goto disable_scm_bw;
>  	}
>  
>  	*output_rt_size = size;
> -	qcom_tzmem_free(output_rt_tzm);
> -
> -free_input_rt:
> -	qcom_tzmem_free(input_rt_tzm);
>  
>  disable_scm_bw:
>  	qcom_scm_bw_disable();
> 
> -- 
> 2.47.3
> 

Reviewed-by: Mukesh Ojha <mukesh.ojha@xxxxxxxxxxxxxxxx>

-- 
-Mukesh Ojha