Re: [PATCH v2 06/13] drm/msm/dp: Add support for DP controllers on Nord

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] drm/msm/dp: logic typo in loop condition i < io_start
--

commit 9873fd55a5048fd6867334e37aee81635bc94b5a
Author: Mahadevan P <mahadevan.p@oss.qualcomm.com>

drm/msm/dp: Add support for DP controllers on Nord

This commit adds the Nord DP controller descriptors and compatible strings.
Nord supports four DP controllers, two per MDSS instance.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index bce641d7d8938..d258963cddc64 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -109,6 +109,14 @@ static const struct msm_dp_desc msm_dp_desc_glymur[] = {
>  	{}
>  };
>  
> +static const struct msm_dp_desc msm_dp_desc_nord[] = {
> +	{ .io_start = 0x0af54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
> +	{ .io_start = 0x0af5c000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },
> +	{ .io_start = 0x08d54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
> +	{ .io_start = 0x08d5c000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },
> +	{}
> +};

[Severity: Low]
Does this new DP controller configuration rely on a logic typo in the loop
condition of msm_dp_display_get_desc() to terminate safely?

Looking at msm_dp_display_get_desc(), the loop condition compares the
array index i against the physical memory address io_start:

	for (i = 0; i < descs[i].io_start; i++) {
		if (descs[i].io_start == res->start)
			return &descs[i];
	}

This regression in the loop logic appears to be a typo that should likely be
descs[i].io_start != 0.

The loop avoids crashing because physical memory addresses (like 0x0af54000)
are much larger than the index i, and the zero-initialized sentinel element {}
sets io_start to 0.

Since phys_addr_t is unsigned, checking i < 0 safely evaluates to false and
terminates the loop, but this logic is brittle and semantically incorrect.

Could the loop condition in msm_dp_display_get_desc() be corrected so that
newly added controller arrays do not rely on this unsigned comparison behavior
for termination?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-nord-drv-dt-v2-0-a2ab9af954d2@oss.qualcomm.com?part=6