Re: [PATCH v4 14/16] media: iris: add Gen2 firmware support on the Agatti platform

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `qcm2290_data` struct initialization assigns values to non-existent members `.firmware_desc_gen1` and `.firmware_desc_gen2`, causing a build failure. Additionally, the required `.firmware_desc` member is left uninitialized, leading to a NULL pointer dereference during probe.
--

commit 4f1d2454209dec3530526e228c0b4e78b6170ab3
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>

media: iris: add Gen2 firmware support on the Agatti platform

Agatti platform is using HFI Gen1 firmware, which is considered to be
legacy firmware branch. Follow the example of the SC7280 platform and
extend the driver with supporting both HFI Gen1 and Gen2 firmwares for
this platform. Like HFI Gen1 this firmware doesn't have PIPE property
(but unlike Gen1 buffer sizes are calculated on the driver side).

> diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c b/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
> index 393256f39112b..d9de7dcb59e3a 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu_ar50lt.c
[ ... ]
> @@ -78,7 +84,8 @@ static struct platform_inst_caps platform_inst_cap_ar50lt = {
>  };
>  
>  const struct iris_platform_data qcm2290_data = {
> -	.firmware_desc_gen1 = &iris_vpu_ar50lt_p1_gen1_s6_desc,
> +	.firmware_desc_gen1 = &iris_vpu_ar50lt_p1_gen1_desc,
> +	.firmware_desc_gen2 = &iris_vpu_ar50lt_p1_gen2_s6_desc,

[Severity: Low]
Does this cause a build failure? struct iris_platform_data does not
currently contain a firmware_desc_gen2 field (nor a firmware_desc_gen1
field).

Looking at drivers/media/platform/qcom/iris/iris_platform_common.h:

struct iris_platform_data {
	/*
	 * XXX: replace with gen1 / gen2 pointers once we have platforms
	 * supporting both firmware kinds.
	 */
	const struct iris_firmware_desc *firmware_desc;

If these fields were added to bypass the build failure, wouldn't this
initialization leave the required firmware_desc field uninitialized?
This could result in a null pointer dereference during probe:

drivers/media/platform/qcom/iris/iris_probe.c:iris_probe()
	core->iris_firmware_desc = core->iris_platform_data->firmware_desc;
	core->iris_firmware_data = core->iris_firmware_desc->firmware_data;

>  	.vpu_ops = &iris_vpu_ar50lt_ops,
>  	.icc_tbl = iris_icc_info_ar50lt,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-iris-ar50lt-v4-0-0abfb74d5b3c@oss.qualcomm.com?part=14