Re: [PATCH v8 08/12] media: iris: Handle CPU_CS_SCIACMDARG3 register write via program bootup registers hook

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The error path for checking `CTRL_ERROR_STATUS__M` breaks out of the timeout loop but erroneously returns success.
--

commit c710baa8452aee4bafd7c2c310b77e43ce0faf2f
Author: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

media: iris: Handle CPU_CS_SCIACMDARG3 register write via program bootup registers hook

This commit moves the CPU_CS_SCIACMDARG3 register write into the bootup
registers hook and provides a platform-specific value for the Glymur
platform to avoid firmware miscalculating the VM count.

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> index 330fc85d232b4..a42554272964e 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
[ ... ]
> @@ -78,7 +79,6 @@ int iris_vpu_boot_firmware(struct iris_core *core)
>  	iris_vpu_setup_ucregion_memory_map(core);
>  
>  	writel(ctrl_init, core->reg_base + CTRL_INIT);
> -	writel(0x1, core->reg_base + CPU_CS_SCIACMDARG3);
>  
>  	while (!ctrl_status && count < max_tries) {
>  		ctrl_status = readl(core->reg_base + CTRL_STATUS);

[Severity: High]
This is a pre-existing issue, but I noticed something concerning in the
error handling within iris_vpu_boot_firmware.

If the firmware initialization sets the error bit, the code logs an error
and breaks out of the polling loop:

    if ((ctrl_status & CTRL_ERROR_STATUS__M) == 0x4) {
        dev_err(core->dev, "invalid setting for uc_region\n");
        break;
    }

Since the loop is broken early, the loop counter remains less than the
maximum number of tries. The subsequent timeout check evaluates to false:

    if (count >= max_tries) {
        dev_err(core->dev, "error booting up iris firmware\n");
        return -ETIME;
    }

This causes the function to proceed with enabling interrupts and eventually
return success instead of failing early.

Does this logic cause the driver to ignore a fatal firmware error and
leave the hardware in an invalid state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-glymur-v8-0-1c79b9d51fc0@oss.qualcomm.com?part=8