Hi Prabhakar, Thank you for the patch. On Mon, Apr 28, 2025 at 10:52:08AM +0100, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > Simplify the `rzg2l_fifo_empty()` helper by removing the redundant > comparison in the return path. Now the function explicitly returns `true` > if the FIFO write and read pointers match, and `false` otherwise, improving > readability without changing behavior. > > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > Closes: https://lore.kernel.org/all/aAtQThCibZCROETx@stanley.mountain/ > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > --- > drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > index 067c6af14e95..97faefcd6019 100644 > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c > @@ -348,7 +348,7 @@ bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru) > if (amnfifopntr_w == amnfifopntr_r_y) > return true; > > - return amnfifopntr_w == amnfifopntr_r_y; > + return false; So the function always returned true. This seems to be a bug fix, please add a Fixes: tag. The commit message should also make it clear that you're fixing an issue, not just simplifying the code. Personally I'd have written diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c index 067c6af14e95..3d0810b3c35e 100644 --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c @@ -345,8 +345,6 @@ bool rzg2l_fifo_empty(struct rzg2l_cru_dev *cru) amnfifopntr_w = amnfifopntr & AMnFIFOPNTR_FIFOWPNTR; amnfifopntr_r_y = (amnfifopntr & AMnFIFOPNTR_FIFORPNTR_Y) >> 16; - if (amnfifopntr_w == amnfifopntr_r_y) - return true; return amnfifopntr_w == amnfifopntr_r_y; } but that's also a bit of a style preference. > } > > void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru) -- Regards, Laurent Pinchart
