Hi Dan,
Thank you for the comments.
On Friday 04 October 2013 11:16:38 Dan Carpenter wrote:
> On Thu, Oct 03, 2013 at 01:55:29AM +0200, Laurent Pinchart wrote:
> > +
> > + ret = vb2_streamon(&vfh->queue, type);
> > + if (ret < 0)
> > + goto err_iss_video_check_format;
> > +
> > + /* In sensor-to-memory mode, the stream can be started synchronously
> > + * to the stream on command. In memory-to-memory mode, it will be
> > + * started when buffers are queued on both the input and output.
> > + */
> > + if (pipe->input == NULL) {
> > + unsigned long flags;
> > + ret = omap4iss_pipeline_set_stream(pipe,
> > + ISS_PIPELINE_STREAM_CONTINUOUS);
> > + if (ret < 0)
> > + goto err_omap4iss_set_stream;
> > + spin_lock_irqsave(&video->qlock, flags);
> > + if (list_empty(&video->dmaqueue))
> > + video->dmaqueue_flags |= ISS_VIDEO_DMAQUEUE_UNDERRUN;
> > + spin_unlock_irqrestore(&video->qlock, flags);
> > + }
> > +
> > + if (ret < 0) {
> > +err_omap4iss_set_stream:
> > + vb2_streamoff(&vfh->queue, type);
> > +err_iss_video_check_format:
> > + media_entity_pipeline_stop(&video->video.entity);
> > +err_media_entity_pipeline_start:
> > + if (video->iss->pdata->set_constraints)
> > + video->iss->pdata->set_constraints(video->iss, false);
> > + video->queue = NULL;
> > + }
> > +
> > + if (!ret)
> > + video->streaming = 1;
> > +
> > + mutex_unlock(&video->stream_lock);
> > + return ret;
> > +}
>
> This driver is mostly really nice code, but this error handling is
> spaghetti-al-nasty. Just split up the success and failure returns.
>
> video->streaming = 1;
> mutex_unlock(&video->stream_lock);
> return 0;
>
> err_omap4iss_set_stream:
> vb2_streamoff(&vfh->queue, type);
> err_iss_video_check_format:
> media_entity_pipeline_stop(&video->video.entity);
> err_media_entity_pipeline_start:
> if (video->iss->pdata->set_constraints)
> video->iss->pdata->set_constraints(video->iss, false);
> video->queue = NULL;
>
> mutex_unlock(&video->stream_lock);
> return ret;
> }
Sure, I'll fix that.
--
Regards,
Laurent Pinchart
--
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
