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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel