Hi Jacopo,
Thanks for your comments.
On 2020-11-16 17:58:14 +0100, Jacopo Mondi wrote:
> Hi Niklas,
>
> On Thu, Nov 12, 2020 at 11:51:46PM +0100, Niklas Söderlund wrote:
> > When a subdevice signals end of stream stop the VIN in addition to
> > informing user-space of the event.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> > drivers/media/platform/rcar-vin/rcar-v4l2.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > index dca3ab1656a66cef..fcaf68c3428b80fd 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -969,9 +969,23 @@ void rvin_v4l2_unregister(struct rvin_dev *vin)
> > static void rvin_notify_video_device(struct rvin_dev *vin,
> > unsigned int notification, void *arg)
> > {
> > + const struct v4l2_event *event;
> > +
>
> Can this go inside the switch ?
It could but I dislike creating 'case FOO: { }' blocks as I think they
are hard to read and un C like ;-P
>
> > switch (notification) {
> > case V4L2_DEVICE_NOTIFY_EVENT:
> > - v4l2_event_queue(&vin->vdev, arg);
> > + event = arg;
> > +
> > + switch (event->type) {
> > + case V4L2_EVENT_EOS:
>
> As there's only a case where this happen, this could be an if, but I
> see a switch is consistent with the existing one. Up to you.
I been bitten by this in the past so now days I go straight for the
switch() construct :-)
>
> Reviewed-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
Thanks!
>
> Thanks
> j
>
> > + rvin_stop_streaming(vin);
> > + v4l2_info(&vin->v4l2_dev,
> > + "Subdevice signaled end of stream, stopping.\n");
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + v4l2_event_queue(&vin->vdev, event);
> > break;
> > default:
> > break;
> > --
> > 2.29.2
> >
--
Regards,
Niklas Söderlund
