[bug report] media: rzg2l-cru: Add function pointer to check if FIFO is empty

[bug report] media: rzg2l-cru: Add function pointer to check if FIFO is empty

[Dan Carpenter]

Hello Lad Prabhakar,

Commit 446c645f7fe4 ("media: rzg2l-cru: Add function pointer to check
if FIFO is empty") from Apr 11, 2025 (linux-next), leads to the
following Smatch static checker warning:

	drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c:351 rzg2l_fifo_empty()
	warn: duplicate check 'amnfifopntr_w == amnfifopntr_r_y' (previous on line 348)

drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
    339 bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru)
    340 {
    341         u32 amnfifopntr, amnfifopntr_w, amnfifopntr_r_y;
    342 
    343         amnfifopntr = rzg2l_cru_read(cru, AMnFIFOPNTR);
    344 
    345         amnfifopntr_w = amnfifopntr & AMnFIFOPNTR_FIFOWPNTR;
    346         amnfifopntr_r_y =
    347                 (amnfifopntr & AMnFIFOPNTR_FIFORPNTR_Y) >> 16;
    348         if (amnfifopntr_w == amnfifopntr_r_y)
    349                 return true;

We should either delete this check

    350 
--> 351         return amnfifopntr_w == amnfifopntr_r_y;

or change this to "return false;".  (I prefer the second option).

    352 }

regards,
dan carpenter

Re: [bug report] media: rzg2l-cru: Add function pointer to check if FIFO is empty

[Lad, Prabhakar]

Hi Dan,

Thank you for the report.

On Fri, Apr 25, 2025 at 10:08 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Hello Lad Prabhakar,
>
> Commit 446c645f7fe4 ("media: rzg2l-cru: Add function pointer to check
> if FIFO is empty") from Apr 11, 2025 (linux-next), leads to the
> following Smatch static checker warning:
>
>         drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c:351 rzg2l_fifo_empty()
>         warn: duplicate check 'amnfifopntr_w == amnfifopntr_r_y' (previous on line 348)
>
> drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
>     339 bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru)
>     340 {
>     341         u32 amnfifopntr, amnfifopntr_w, amnfifopntr_r_y;
>     342
>     343         amnfifopntr = rzg2l_cru_read(cru, AMnFIFOPNTR);
>     344
>     345         amnfifopntr_w = amnfifopntr & AMnFIFOPNTR_FIFOWPNTR;
>     346         amnfifopntr_r_y =
>     347                 (amnfifopntr & AMnFIFOPNTR_FIFORPNTR_Y) >> 16;
>     348         if (amnfifopntr_w == amnfifopntr_r_y)
>     349                 return true;
>
> We should either delete this check
>
>     350
> --> 351         return amnfifopntr_w == amnfifopntr_r_y;
>
> or change this to "return false;".  (I prefer the second option).
>
Agreed, I will go with this.

Cheers,
Prabhakar

>     352 }
>
> regards,
> dan carpenter
>