Re: [PATCH v4 06/16] media: iris: Add platform data field for watchdog interrupt mask

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly introduced `wd_intr_mask` field is not initialized for the `x1p42100_data` platform, causing it to default to 0 and completely breaking watchdog interrupt handling for this platform.
--

commit 0965d657e0a8904cc63b8cbd790144e332d59492
Author: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>

media: iris: Add platform data field for watchdog interrupt mask

This commit introduces the wd_intr_mask platform data field to capture the
watchdog interrupt bitmask per platform. It also replaces hardcoded
watchdog bitmasks with this new field during interrupt handling.

> diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c b/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
> index 2c63adbc55791..aeffd96a4e1cb 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c

[ ... ]

> @@ -208,6 +213,7 @@ const struct iris_platform_data sm8750_data = {
>  	.tz_cp_config_data = tz_cp_config_vpu3,
>  	.tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_vpu3),
>  	.num_vpp_pipe = 4,
> +	.wd_intr_mask = WRAPPER_INTR_STATUS_A2HWD_BMSK,
>  	.max_session_count = 16,
>  	.max_core_mbpf = NUM_MBS_8K * 2,
>  	.max_core_mbps = ((7680 * 4320) / 256) * 60,

[Severity: High]
Does this patch miss updating x1p42100_data?

It appears x1p42100_data is defined further down in this file but does not
have its wd_intr_mask initialized in this commit.

> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> index 375bcd9234766..41498f94480e8 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> @@ -109,11 +109,11 @@ void iris_vpu_raise_interrupt(struct iris_core *core)
>  
>  void iris_vpu_clear_interrupt(struct iris_core *core)
>  {
> +	u32 wd_intr_mask = core->iris_platform_data->wd_intr_mask;
>  	u32 intr_status, mask;
>  
>  	intr_status = readl(core->reg_base + WRAPPER_INTR_STATUS);
> -	mask = (WRAPPER_INTR_STATUS_A2H_BMSK |
> -		WRAPPER_INTR_STATUS_A2HWD_BMSK |
> +	mask = (WRAPPER_INTR_STATUS_A2H_BMSK | wd_intr_mask |
>  		CTRL_INIT_IDLE_MSG_BMSK);
>  
>  	if (intr_status & mask)

[Severity: High]
Since x1p42100_data is a static global variable, its uninitialized
wd_intr_mask will default to 0.

Will this cause the watchdog interrupt bit to be omitted from this mask,
preventing watchdog interrupts from being captured into core->intr_status
for the x1p42100 platform?

> @@ -124,7 +124,9 @@ void iris_vpu_clear_interrupt(struct iris_core *core)
>  
>  int iris_vpu_watchdog(struct iris_core *core, u32 intr_status)
>  {
> -	if (intr_status & WRAPPER_INTR_STATUS_A2HWD_BMSK) {
> +	u32 wd_intr_mask = core->iris_platform_data->wd_intr_mask;
> +
> +	if (intr_status & wd_intr_mask) {
>  		dev_err(core->dev, "received watchdog interrupt\n");
>  		return -ETIME;
>  	}

[Severity: High]
Similarly, will this cause intr_status & wd_intr_mask to always evaluate
to 0 for x1p42100, silently dropping all watchdog timeouts and breaking
firmware hang recovery?

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