Re: [PATCH v2 1/5] media: i2c: vd55g1: Fix media bus code initialization

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Meaningful code and commit message mismatch: The commit message states 'Rename VD55G1_MBUS_CODE_DEF to VD55G1_MBUS_CODE_IDX_DEF', but the macro is deleted instead and hardcoded '0' indices are used.
- [Low] Type mismatch: The `code` variable is declared as a signed `int` but is used to store V4L2 media bus format codes, which are strictly unsigned 32-bit integers (`u32`).
--

commit 6cf5089e88c71a68129dc09d9aa82be4ff08d4b3
Author: Benjamin Mugnier <benjamin.mugnier@foss.st.com>

media: i2c: vd55g1: Fix media bus code initialization

This commit passes the actual media bus code to vd55g1_get_fmt_code()
during driver initialization instead of the array index. It also renames
macro definitions to clarify their usage as indices.

> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
> index 78d18c0281541..fceb437e19bec 100644
> --- a/drivers/media/i2c/vd55g1.c
> +++ b/drivers/media/i2c/vd55g1.c

[ ... ]

> @@ -1347,6 +1346,7 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>  {
>  	struct vd55g1 *sensor = to_vd55g1(sd);
>  	struct v4l2_subdev_format fmt = { 0 };
> +	int code;

[Severity: Low]
Does this type mismatch cause any issues? V4L2 media bus format codes are
strictly unsigned 32-bit integers (u32), but this is declared as a signed int.

>  	struct v4l2_subdev_route routes[] = {
>  		{ .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE }
>  	};
> @@ -1361,9 +1361,13 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>  	if (ret)
>  		return ret;
>  
> -	vd55g1_update_pad_fmt(sensor, &vd55g1_supported_modes[VD55G1_MODE_DEF],
> -			      vd55g1_get_fmt_code(sensor, VD55G1_MBUS_CODE_DEF),
> -			      &fmt.format);
> +	if (sensor->id == VD55G1_MODEL_ID_VD55G1)
> +		code = vd55g1_mbus_formats_mono[0];
> +	else
> +		code = vd55g1_mbus_formats_bayer[0][0];

[Severity: Medium]
The commit message states "Rename VD55G1_MBUS_CODE_DEF to
VD55G1_MBUS_CODE_IDX_DEF", but the macro was deleted entirely in this patch.

Should these hardcoded 0 indices use the promised VD55G1_MBUS_CODE_IDX_DEF
macro instead to avoid confusion?

> +	vd55g1_update_pad_fmt(sensor,
> +			      &vd55g1_supported_modes[VD55G1_MODE_IDX_DEF],
> +			      vd55g1_get_fmt_code(sensor, code), &fmt.format);
>  
>  	return vd55g1_set_pad_fmt(sd, sd_state, &fmt);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-vd55g4_and_fixes-v2-0-ea0a7cf3cd66@foss.st.com?part=1