Hi Hans,
Em Wed, 16 Apr 2014 18:38:25 -0300
Mauro Carvalho Chehab <m.chehab@xxxxxxxxxxx> escreveu:
> Em Fri, 11 Apr 2014 10:11:07 +0200
> Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
>
> > From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >
> > --- a/drivers/media/platform/blackfin/bfin_capture.c
> > +++ b/drivers/media/platform/blackfin/bfin_capture.c
> > @@ -427,15 +427,12 @@ static int bcap_start_streaming(struct vb2_queue *vq, unsigned int count)
> > return 0;
> > }
> >
> > -static int bcap_stop_streaming(struct vb2_queue *vq)
> > +static void bcap_stop_streaming(struct vb2_queue *vq)
> > {
> > struct bcap_device *bcap_dev = vb2_get_drv_priv(vq);
> > struct ppi_if *ppi = bcap_dev->ppi;
> > int ret;
> >
> > - if (!vb2_is_streaming(vq))
> > - return 0;
> > -
>
> Why are you dropping this? IMHO, you should be doing, instead:
> if (!vb2_is_streaming(vq))
> return;
>
> Except if you're 100% sure that checking it here can be removed. On
> this case, please put this on a separate patch, clearly explaining
> why we can safely remove this.
>
> Please notice that on other similar parts of this patch, you didn't remove
> the test, just removed the returned parameter.
As I said before, if you need to remove the vb2_is_streaming(vq) check
above, please do it on a separate patch, properly justifying why you're
doing that.
This hunk is still present on your new pull request.
--
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
