Re: [PATCH v4 04/19] firmware: qcom: scm: use dev_err_probe() for dload address failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The global `qcom_tzmem_dev` pointer is set during `qcom_tzmem_enable()` but is never cleared on probe failure, causing subsequent probe retries to permanently fail with `-EBUSY`.
- [High] The probe function calls `of_reserved_mem_device_init()` but does not call `of_reserved_mem_device_release()` on subsequent error paths, leading to permanent probe failure or resource leaks upon probe retry.
--

commit 6dc15526252b78a9c217f06ed35ed4966a794450
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

firmware: qcom: scm: use dev_err_probe() for dload address failure

Replace the bare return ret after qcom_scm_find_dload_address() with
dev_err_probe() to produce a consistent, deferred-probe-aware error
message when the download-mode address cannot be resolved.

> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index c8a84836887f0..6cd4d9468456f 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -2750,7 +2750,8 @@ static int qcom_scm_probe(struct platform_device *pdev)
>  	scm->dev = &pdev->dev;
>  	ret = qcom_scm_find_dload_address(&pdev->dev, &scm->dload_mode_addr);
>  	if (ret < 0)
> -		return ret;
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "Failed to get download mode address\n");
>  
>  	mutex_init(&scm->scm_bw_lock);

[Severity: High]
This is a pre-existing issue, but does qcom_scm_probe() properly release
resources on later probe failures?

Later in qcom_scm_probe(), of_reserved_mem_device_init() is called to setup
reserved memory:

drivers/firmware/qcom/qcom_scm.c:qcom_scm_probe() {
	...
	ret = of_reserved_mem_device_init(scm->dev);
	if (ret && ret != -ENODEV)
		return dev_err_probe(scm->dev, ret,
				     "Failed to setup the reserved memory region for TZ mem\n");
	...
}

However, subsequent error paths, such as platform_get_irq_optional() returning
-EPROBE_DEFER, exit directly without calling of_reserved_mem_device_release().

If the probe is deferred, will this cause the next probe attempt to fail with
-EBUSY, or leak memory by duplicating the rmem_assigned_device list entry on
every retry?

[Severity: High]
This is a pre-existing issue, but does qcom_tzmem_enable() cause a permanent
probe failure if qcom_scm_probe() defers later in the function?

qcom_tzmem_enable() sets a static qcom_tzmem_dev pointer in qcom_tzmem.c:

drivers/firmware/qcom/qcom_tzmem.c:qcom_tzmem_enable() {
	if (qcom_tzmem_dev)
		return -EBUSY;

	qcom_tzmem_dev = dev;

	return qcom_tzmem_init();
}

Since qcom_scm_probe() does not clear this pointer if it defers after this
call, will subsequent probe retries permanently fail with -EBUSY when
qcom_tzmem_enable() checks the still-set pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624190830.3131112-1-mukesh.ojha@oss.qualcomm.com?part=4