Messages in this thread | | | Date | Tue, 31 Oct 2023 10:46:52 +0100 | Subject | Re: [PATCH v9 2/3] media: rockchip: Add a driver for Rockchip's camera interface | From | Christophe JAILLET <> |
| |
Le 31/10/2023 à 10:33, Mehdi Djait a écrit : > Hello Christophe, > > On Mon, Oct 30, 2023 at 01:47:17PM +0100, Christophe JAILLET wrote: >>> + /* Create & register platform subdev. */ >>> + ret = cif_register_stream_vdev(cif_dev); >>> + if (ret < 0) >>> + goto err_unreg_media_dev; >>> + >>> + ret = cif_subdev_notifier(cif_dev); >>> + if (ret < 0) { >>> + v4l2_err(&cif_dev->v4l2_dev, >>> + "Failed to register subdev notifier(%d)\n", ret); >>> + cif_unregister_stream_vdev(cif_dev); >>> + goto err_unreg_media_dev; >> >> Should there be another label with cif_unregister_stream_vdev(cif_dev); if >> an error occurs here? >> >> CJ > > cif_subdev_notifier() is the last function call in the probe with error > handling. So it will undo the last successful register: > cif_register_stream_vdev and use the goto to unregister the rest.
Ah, I didn't see the cif_unregister_stream_vdev() call here. Sorry for the noise.
> > I can add a label err_unreg_stream_vdev but it will be only used in the > error handling of cif_subdev_notifier() and I don't see the benefit.
The main benefit is to be more consistent in the way the error path is written, and to be more future proof.
CJ > > -- > Kind Regards > Mehdi Djait > >>> + } >>> + >>> + cif_set_default_format(cif_dev); >>> + pm_runtime_enable(&pdev->dev); >>> + >>> + return 0; >>> + >>> +err_unreg_media_dev: >>> + media_device_unregister(&cif_dev->media_dev); >>> +err_unreg_v4l2_dev: >>> + v4l2_device_unregister(&cif_dev->v4l2_dev); >>> + return ret; >>> +} >
| |