Re: [PATCH] drm/bridge: sii902x: Read "sil,i2s-data-lanes" as u32

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Using WARN_ON() to validate device tree input causes unnecessary kernel splats and potential kernel panics.
- [Medium] Breaking out of the initialization loop on invalid lane values leaves the audio data structures in an inconsistent state.
--

--- Patch [1]: [PATCH] drm/bridge: sii902x: Read "sil,i2s-data-lanes" as u32 ---
commit a4b4eceb945fa6ac0b81acc248b81fdebf849bd9
Author: Rob Herring (Arm) <[email protected]>

drm/bridge: sii902x: Read "sil,i2s-data-lanes" as u32

This patch updates the sii902x bridge driver to read the "sil,i2s-data-lanes"
property as a u32 array instead of a u8 array to match the device tree
bindings. It also introduces a bounds check on the array values to prevent
out-of-bounds accesses.

> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
[ ... ]
> @@ -890,9 +890,12 @@ static int sii902x_audio_codec_init(struct sii902x *sii902x,
>  	}
>  	codec_data.max_i2s_channels = 2 * num_lanes;
>  
> -	for (i = 0; i < num_lanes; i++)
> +	for (i = 0; i < num_lanes; i++) {
> +		if (WARN_ON(lanes[i] >= ARRAY_SIZE(i2s_lane_id)))

[Severity: Medium]
Is it safe to use WARN_ON() for validating device tree properties?

If panic_on_warn is enabled, couldn't this crash the system when parsing
malformed or untrusted device tree data?

Would it be better to use dev_err() and gracefully fail initialization?

[Severity: Medium]
If this loop breaks early due to an invalid lane, codec_data.max_i2s_channels
remains set to 2 * num_lanes.

Could this cause an inconsistent state where ALSA exposes support for more
channels than the hardware has actually configured in i2s_fifo_sequence?

Should this path return an error (like -EINVAL) to abort audio
initialization, or update the channel count?

> +			break;
>  		sii902x->audio.i2s_fifo_sequence[i] |= audio_fifo_id[i] |
>  			i2s_lane_id[lanes[i]] |	SII902X_TPI_I2S_FIFO_ENABLE;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1