Re: [PATCH 4/9] media: vimc: Ensure that pixel_rate fits in 32 bits

[Faizel K B]

On Mon, Jun 29, 2026 at 11:30:45AM +0000, Ricardo Ribalda wrote:
> pixel_rate is set to VIMC_PIXEL_RATE_FIXED, which the code expects to
> fit in 32 bits. Make that constraint into a WARN_ON, so if we ever break
> that constraint a kernel warning will be triggered.
>
> It also fixes the following cocci warning:
> ./test-drivers/vimc/vimc-sensor.c:107:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
>
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> drivers/media/test-drivers/vimc/vimc-sensor.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
> index d125a79fec8c..83dcc9d61ee0 100644
> --- a/drivers/media/test-drivers/vimc/vimc-sensor.c
> +++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
> @@ -103,8 +103,12 @@ static void vimc_sensor_update_frame_timing(struct v4l2_subdev *sd,
> 	u64 total_pixels = (u64)hts * vts;
> 	u64 frame_interval_ns;
>
> +	/* Sanity check, pixel rate is fixed and fits in 32 bits. */
> +	if (WARN_ON(pixel_rate >= 0x100000000))
> +		return;
> +
> 	frame_interval_ns = total_pixels * NSEC_PER_SEC;
> -	do_div(frame_interval_ns, pixel_rate);
> +	do_div(frame_interval_ns, (u32)pixel_rate);

There was an error in the data type of pixel_rate. Actually it is u32 instead
of u64. There are two changes suggested by Geert in reply to the original
report. v4l2_ctrl.val is s32, not u64 and using mul_u32_u32() for total_pixels
calculation. We can avoid the sanity check also. Would you be able to consider
these two changes which avoid the warning?

-       u64 pixel_rate = vsensor->pixel_rate->val;
+       u32 pixel_rate = vsensor->pixel_rate->val;
        u32 hts = width + vsensor->hblank->val;
        u32 vts = height + vsensor->vblank->val;
-       u64 total_pixels = (u64)hts * vts;
+       u64 total_pixels = mul_u32_u32(hts, vts);
        u64 frame_interval_ns;

Link for the suggested changes
https://lore.kernel.org/lkml/CAMuHMdVx80Eh8w5-OioH-Uyahea0ZRQHUVg9MBfQ4uEjaxM5Lg@xxxxxxxxxxxxxx/