Hi Yong, On Fri, Mar 30, 2018 at 11:15 AM Yong Zhi <yong.zhi@xxxxxxxxx> wrote: [snip] > +static int imgu_video_nodes_init(struct imgu_device *imgu) > +{ > + struct v4l2_pix_format_mplane *fmts[IPU3_CSS_QUEUES] = { NULL }; > + struct v4l2_rect *rects[IPU3_CSS_RECTS] = { NULL }; > + unsigned int i; > + int r; > + > + imgu->buf_struct_size = sizeof(struct imgu_buffer); > + > + for (i = 0; i < IMGU_NODE_NUM; i++) { > + imgu->nodes[i].name = imgu_node_map[i].name; > + imgu->nodes[i].output = i < IMGU_QUEUE_FIRST_INPUT; > + imgu->nodes[i].immutable = false; > + imgu->nodes[i].enabled = false; > + > + if (i != IMGU_NODE_PARAMS && i != IMGU_NODE_STAT_3A) > + fmts[imgu_node_map[i].css_queue] = > + &imgu->nodes[i].vdev_fmt.fmt.pix_mp; > + atomic_set(&imgu->nodes[i].sequence, 0); > + } > + > + /* Master queue is always enabled */ > + imgu->nodes[IMGU_QUEUE_MASTER].immutable = true; > + imgu->nodes[IMGU_QUEUE_MASTER].enabled = true; > + > + r = ipu3_v4l2_register(imgu); > + if (r) > + return r; > + > + /* Set initial formats and initialize formats of video nodes */ > + rects[IPU3_CSS_RECT_EFFECTIVE] = &imgu->rect.eff; > + rects[IPU3_CSS_RECT_BDS] = &imgu->rect.bds; > + ipu3_css_fmt_set(&imgu->css, fmts, rects); > + > + /* Pre-allocate dummy buffers */ > + r = imgu_dummybufs_preallocate(imgu); > + if (r) { > + dev_err(&imgu->pci_dev->dev, > + "failed to pre-allocate dummy buffers (%d)", r); > + imgu_dummybufs_cleanup(imgu); No need to call ipu3_v4l2_unregister() here? (That's why I keep suggesting use of single return error path with labels named after the first cleanup step that needs to be done, as it makes it easier to spot such mistakes.) > + return r; > + } > + > + return 0; > +} > + > +static void imgu_video_nodes_exit(struct imgu_device *imgu) > +{ > + imgu_dummybufs_cleanup(imgu); > + ipu3_v4l2_unregister(imgu); > +} Best regards, Tomasz