Re: [PATCH 2/2] [media] media-device: split media initialization and registration

From: Sakari Ailus
Date: Thu Sep 10 2015 - 13:16:57 EST


Hi Javier,

Thanks for the set! A few comments below.

Javier Martinez Canillas wrote:
> The media device node is registered and so made visible to user-space
> before entities are registered and links created which means that the
> media graph obtained by user-space could be only partially enumerated
> if that happens too early before all the graph has been created.
>
> To avoid this race condition, split the media init and registration
> in separate functions and only register the media device node when
> all the pending subdevices have been registered, either explicitly
> by the driver or asynchronously using v4l2_async_register_subdev().
>
> Also, add a media_entity_cleanup() function that will destroy the
> graph_mutex that is initialized in media_entity_init().
>
> Suggested-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Signed-off-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
>
> ---
>
> drivers/media/common/siano/smsdvb-main.c | 1 +
> drivers/media/media-device.c | 38 +++++++++++++++++++++++----
> drivers/media/platform/exynos4-is/media-dev.c | 12 ++++++---
> drivers/media/platform/omap3isp/isp.c | 11 +++++---
> drivers/media/platform/s3c-camif/camif-core.c | 13 ++++++---
> drivers/media/platform/vsp1/vsp1_drv.c | 19 ++++++++++----
> drivers/media/platform/xilinx/xilinx-vipp.c | 11 +++++---
> drivers/media/usb/au0828/au0828-core.c | 26 +++++++++++++-----
> drivers/media/usb/cx231xx/cx231xx-cards.c | 22 +++++++++++-----
> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 11 +++++---
> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 13 ++++++---
> drivers/media/usb/siano/smsusb.c | 14 ++++++++--
> drivers/media/usb/uvc/uvc_driver.c | 9 +++++--
> include/media/media-device.h | 2 ++
> 14 files changed, 156 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/media/common/siano/smsdvb-main.c b/drivers/media/common/siano/smsdvb-main.c
> index ab345490a43a..8a1ea2192439 100644
> --- a/drivers/media/common/siano/smsdvb-main.c
> +++ b/drivers/media/common/siano/smsdvb-main.c
> @@ -617,6 +617,7 @@ static void smsdvb_media_device_unregister(struct smsdvb_client_t *client)
> if (!coredev->media_dev)
> return;
> media_device_unregister(coredev->media_dev);
> + media_device_cleanup(coredev->media_dev);
> kfree(coredev->media_dev);
> coredev->media_dev = NULL;
> #endif
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 745defb34b33..a8beb0b445a6 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -526,7 +526,7 @@ static void media_device_release(struct media_devnode *mdev)
> }
>
> /**
> - * media_device_register - register a media device
> + * media_device_init() - initialize a media device
> * @mdev: The media device
> *
> * The caller is responsible for initializing the media device before
> @@ -534,12 +534,11 @@ static void media_device_release(struct media_devnode *mdev)
> *
> * - dev must point to the parent device
> * - model must be filled with the device model name
> + *
> + * returns zero on success or a negative error code.
> */
> -int __must_check __media_device_register(struct media_device *mdev,
> - struct module *owner)
> +int __must_check media_device_init(struct media_device *mdev)

I think I suggested making media_device_init() return void as the only
remaining source of errors would be driver bugs.

I'd simply replace the WARN_ON() below with BUG().

> {
> - int ret;
> -
> if (WARN_ON(mdev->dev == NULL || mdev->model[0] == 0))
> return -EINVAL;
>
> @@ -550,6 +549,35 @@ int __must_check __media_device_register(struct media_device *mdev,
> spin_lock_init(&mdev->lock);
> mutex_init(&mdev->graph_mutex);
>
> + dev_dbg(mdev->dev, "Media device initialized\n");
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(media_device_init);
> +
> +/**
> + * media_device_cleanup() - Cleanup a media device
> + * @mdev: The media device
> + *
> + */
> +void media_device_cleanup(struct media_device *mdev)
> +{
> + mutex_destroy(&mdev->graph_mutex);

Very nice!

> +}
> +EXPORT_SYMBOL_GPL(media_device_cleanup);
> +
> +/**
> + * __media_device_register() - register a media device
> + * @mdev: The media device
> + * @owner: The module owner
> + *
> + * returns zero on success or a negative error code.
> + */
> +int __must_check __media_device_register(struct media_device *mdev,
> + struct module *owner)
> +{
> + int ret;
> +
> /* Register the device node. */
> mdev->devnode.fops = &media_device_fops;
> mdev->devnode.parent = mdev->dev;
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index 4a25df9dd869..158738bd23fc 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -1313,7 +1313,10 @@ static int subdev_notifier_complete(struct v4l2_async_notifier *notifier)
> ret = v4l2_device_register_subdev_nodes(&fmd->v4l2_dev);
> unlock:
> mutex_unlock(&fmd->media_dev.graph_mutex);
> - return ret;
> + if (ret < 0)
> + return ret;
> +
> + return media_device_register(&fmd->media_dev);

Uh-oh.

I guess it's fair to simply fail here. probe() is already over so I
wonder if there's anything we could really do except await someone
unloading and loading the module again? Or, replugging the hardware.

I don't like the idea but I guess this could be solved later on.

> }
>
> static int fimc_md_probe(struct platform_device *pdev)
> @@ -1350,9 +1353,9 @@ static int fimc_md_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ret = media_device_register(&fmd->media_dev);
> + ret = media_device_init(&fmd->media_dev);
> if (ret < 0) {
> - v4l2_err(v4l2_dev, "Failed to register media device: %d\n", ret);
> + v4l2_err(v4l2_dev, "Failed to init media device: %d\n", ret);
> goto err_v4l2_dev;
> }
>
> @@ -1424,7 +1427,7 @@ err_clk:
> err_m_ent:
> fimc_md_unregister_entities(fmd);
> err_md:
> - media_device_unregister(&fmd->media_dev);
> + media_device_cleanup(&fmd->media_dev);
> err_v4l2_dev:
> v4l2_device_unregister(&fmd->v4l2_dev);
> return ret;
> @@ -1445,6 +1448,7 @@ static int fimc_md_remove(struct platform_device *pdev)
> fimc_md_unregister_entities(fmd);
> fimc_md_pipelines_free(fmd);
> media_device_unregister(&fmd->media_dev);
> + media_device_cleanup(&fmd->media_dev);
> fimc_md_put_clocks(fmd);
>
> return 0;
> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> index cb8ac90086c1..b7eda3043c31 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -1793,6 +1793,7 @@ static void isp_unregister_entities(struct isp_device *isp)
>
> v4l2_device_unregister(&isp->v4l2_dev);
> media_device_unregister(&isp->media_dev);
> + media_device_cleanup(&isp->media_dev);
> }
>
> static int isp_link_entity(
> @@ -1875,9 +1876,9 @@ static int isp_register_entities(struct isp_device *isp)
> sizeof(isp->media_dev.model));
> isp->media_dev.hw_revision = isp->revision;
> isp->media_dev.link_notify = isp_pipeline_link_notify;
> - ret = media_device_register(&isp->media_dev);
> + ret = media_device_init(&isp->media_dev);
> if (ret < 0) {
> - dev_err(isp->dev, "%s: Media device registration failed (%d)\n",
> + dev_err(isp->dev, "%s: Media device init failed (%d)\n",
> __func__, ret);
> return ret;
> }
> @@ -2347,7 +2348,11 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> }
> }
>
> - return v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> + ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> + if (ret < 0)
> + return ret;
> +
> + return media_device_register(&isp->media_dev);
> }
>
> /*
> diff --git a/drivers/media/platform/s3c-camif/camif-core.c b/drivers/media/platform/s3c-camif/camif-core.c
> index 3e33c60be004..1428db2f804d 100644
> --- a/drivers/media/platform/s3c-camif/camif-core.c
> +++ b/drivers/media/platform/s3c-camif/camif-core.c
> @@ -305,7 +305,7 @@ static void camif_unregister_media_entities(struct camif_dev *camif)
> /*
> * Media device
> */
> -static int camif_media_dev_register(struct camif_dev *camif)
> +static int camif_media_dev_init(struct camif_dev *camif)
> {
> struct media_device *md = &camif->media_dev;
> struct v4l2_device *v4l2_dev = &camif->v4l2_dev;
> @@ -328,7 +328,7 @@ static int camif_media_dev_register(struct camif_dev *camif)
> if (ret < 0)
> return ret;
>
> - ret = media_device_register(md);
> + ret = media_device_init(md);
> if (ret < 0)
> v4l2_device_unregister(v4l2_dev);
>
> @@ -483,7 +483,7 @@ static int s3c_camif_probe(struct platform_device *pdev)
> goto err_alloc;
> }
>
> - ret = camif_media_dev_register(camif);
> + ret = camif_media_dev_init(camif);
> if (ret < 0)
> goto err_mdev;
>
> @@ -510,6 +510,11 @@ static int s3c_camif_probe(struct platform_device *pdev)
> goto err_unlock;
>
> mutex_unlock(&camif->media_dev.graph_mutex);
> +
> + ret = media_device_register(&camif->media_dev);
> + if (ret < 0)
> + goto err_sens;
> +
> pm_runtime_put(dev);
> return 0;
>
> @@ -518,6 +523,7 @@ err_unlock:
> err_sens:
> v4l2_device_unregister(&camif->v4l2_dev);
> media_device_unregister(&camif->media_dev);
> + media_device_cleanup(&camif->media_dev);
> camif_unregister_media_entities(camif);
> err_mdev:
> vb2_dma_contig_cleanup_ctx(camif->alloc_ctx);
> @@ -539,6 +545,7 @@ static int s3c_camif_remove(struct platform_device *pdev)
> struct s3c_camif_plat_data *pdata = &camif->pdata;
>
> media_device_unregister(&camif->media_dev);
> + media_device_cleanup(&camif->media_dev);
> camif_unregister_media_entities(camif);
> v4l2_device_unregister(&camif->v4l2_dev);
>
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
> index 8f995d267646..bcbc24e55bf5 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -127,6 +127,7 @@ static void vsp1_destroy_entities(struct vsp1_device *vsp1)
>
> v4l2_device_unregister(&vsp1->v4l2_dev);
> media_device_unregister(&vsp1->media_dev);
> + media_device_cleanup(&vsp1->media_dev);
> }
>
> static int vsp1_create_entities(struct vsp1_device *vsp1)
> @@ -141,9 +142,9 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
> strlcpy(mdev->model, "VSP1", sizeof(mdev->model));
> snprintf(mdev->bus_info, sizeof(mdev->bus_info), "platform:%s",
> dev_name(mdev->dev));
> - ret = media_device_register(mdev);
> + ret = media_device_init(mdev);
> if (ret < 0) {
> - dev_err(vsp1->dev, "media device registration failed (%d)\n",
> + dev_err(vsp1->dev, "media device init failed (%d)\n",
> ret);
> return ret;
> }
> @@ -288,11 +289,19 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
> }
>
> ret = v4l2_device_register_subdev_nodes(&vsp1->v4l2_dev);
> -
> -done:
> if (ret < 0)
> - vsp1_destroy_entities(vsp1);
> + goto done;
>
> + ret = media_device_register(mdev);

Are there cases where media_device_register() won't complain aloud
already? I wouldn't print an error message in a driver anymore.

> + if (ret < 0) {
> + dev_err(vsp1->dev, "media device init failed (%d)\n", ret);
> + goto done;
> + }
> +
> + return 0;
> +
> +done:
> + vsp1_destroy_entities(vsp1);
> return ret;
> }
>
> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
> index 79d4be7ce9a5..5bb18aee0707 100644
> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> @@ -311,6 +311,10 @@ static int xvip_graph_notify_complete(struct v4l2_async_notifier *notifier)
> if (ret < 0)
> dev_err(xdev->dev, "failed to register subdev nodes\n");
>
> + ret = media_device_register(&xdev->media_dev);

Same here (and elsewhere).

> + if (ret < 0)
> + dev_err(xdev->dev, "failed to register media device\n");
> +
> return ret;
> }
>
> @@ -571,6 +575,7 @@ static void xvip_composite_v4l2_cleanup(struct xvip_composite_device *xdev)
> {
> v4l2_device_unregister(&xdev->v4l2_dev);
> media_device_unregister(&xdev->media_dev);
> + media_device_cleanup(&xdev->media_dev);
> }
>
> static int xvip_composite_v4l2_init(struct xvip_composite_device *xdev)
> @@ -582,9 +587,9 @@ static int xvip_composite_v4l2_init(struct xvip_composite_device *xdev)
> sizeof(xdev->media_dev.model));
> xdev->media_dev.hw_revision = 0;
>
> - ret = media_device_register(&xdev->media_dev);
> + ret = media_device_init(&xdev->media_dev);
> if (ret < 0) {
> - dev_err(xdev->dev, "media device registration failed (%d)\n",
> + dev_err(xdev->dev, "media device init failed (%d)\n",
> ret);
> return ret;
> }
> @@ -594,7 +599,7 @@ static int xvip_composite_v4l2_init(struct xvip_composite_device *xdev)
> if (ret < 0) {
> dev_err(xdev->dev, "V4L2 device registration failed (%d)\n",
> ret);
> - media_device_unregister(&xdev->media_dev);
> + media_device_cleanup(&xdev->media_dev);
> return ret;
> }
>
> diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
> index 1c12285428fe..c3d7788c2796 100644
> --- a/drivers/media/usb/au0828/au0828-core.c
> +++ b/drivers/media/usb/au0828/au0828-core.c
> @@ -136,6 +136,7 @@ static void au0828_unregister_media_device(struct au0828_dev *dev)
> #ifdef CONFIG_MEDIA_CONTROLLER
> if (dev->media_dev) {
> media_device_unregister(dev->media_dev);
> + media_device_cleanup(dev->media_dev);
> kfree(dev->media_dev);
> dev->media_dev = NULL;
> }
> @@ -214,8 +215,8 @@ static void au0828_usb_disconnect(struct usb_interface *interface)
> au0828_usb_release(dev);
> }
>
> -static void au0828_media_device_register(struct au0828_dev *dev,
> - struct usb_device *udev)
> +static void au0828_media_device_init(struct au0828_dev *dev,
> + struct usb_device *udev)
> {
> #ifdef CONFIG_MEDIA_CONTROLLER
> struct media_device *mdev;
> @@ -237,10 +238,10 @@ static void au0828_media_device_register(struct au0828_dev *dev,
> mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
> mdev->driver_version = LINUX_VERSION_CODE;
>
> - ret = media_device_register(mdev);
> + ret = media_device_init(mdev);
> if (ret) {
> pr_err(
> - "Couldn't create a media device. Error: %d\n",
> + "Couldn't initialize a media device. Error: %d\n",
> ret);
> kfree(mdev);
> return;
> @@ -372,8 +373,8 @@ static int au0828_usb_probe(struct usb_interface *interface,
> dev->boardnr = id->driver_info;
> dev->board = au0828_boards[dev->boardnr];
>
> - /* Register the media controller */
> - au0828_media_device_register(dev, usbdev);
> + /* Initialize the media controller */
> + au0828_media_device_init(dev, usbdev);
>
> #ifdef CONFIG_VIDEO_AU0828_V4L2
> dev->v4l2_dev.release = au0828_usb_v4l2_release;
> @@ -444,9 +445,20 @@ static int au0828_usb_probe(struct usb_interface *interface,
> if (retval) {
> pr_err("%s() au0282_dev_register failed to create graph\n",
> __func__);
> - au0828_usb_disconnect(interface);
> + goto err;
> }
>
> + retval = media_device_register(dev->media_dev);
> + if (retval < 0) {
> + pr_err("%s() au0282_dev_register failed to register media dev\n",
> + __func__);
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + au0828_usb_disconnect(interface);
> return retval;
> }
>
> diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
> index 022b30099a89..fee6dfa9e73e 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
> @@ -1172,6 +1172,7 @@ static void cx231xx_unregister_media_device(struct cx231xx *dev)
> #ifdef CONFIG_MEDIA_CONTROLLER
> if (dev->media_dev) {
> media_device_unregister(dev->media_dev);
> + media_device_cleanup(dev->media_dev);
> kfree(dev->media_dev);
> dev->media_dev = NULL;
> }
> @@ -1205,8 +1206,8 @@ void cx231xx_release_resources(struct cx231xx *dev)
> clear_bit(dev->devno, &cx231xx_devused);
> }
>
> -static void cx231xx_media_device_register(struct cx231xx *dev,
> - struct usb_device *udev)
> +static void cx231xx_media_device_init(struct cx231xx *dev,
> + struct usb_device *udev)
> {
> #ifdef CONFIG_MEDIA_CONTROLLER
> struct media_device *mdev;
> @@ -1224,10 +1225,10 @@ static void cx231xx_media_device_register(struct cx231xx *dev,
> mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
> mdev->driver_version = LINUX_VERSION_CODE;
>
> - ret = media_device_register(mdev);
> + ret = media_device_init(mdev);
> if (ret) {
> dev_err(dev->dev,
> - "Couldn't create a media device. Error: %d\n",
> + "Couldn't initialize a media device. Error: %d\n",
> ret);
> kfree(mdev);
> return;
> @@ -1669,8 +1670,8 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
> /* save our data pointer in this interface device */
> usb_set_intfdata(interface, dev);
>
> - /* Register the media controller */
> - cx231xx_media_device_register(dev, udev);
> + /* Initialize the media controller */
> + cx231xx_media_device_init(dev, udev);
>
> /* Create v4l2 device */
> #ifdef CONFIG_MEDIA_CONTROLLER
> @@ -1744,9 +1745,18 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
> retval = cx231xx_create_media_graph(dev);
> if (retval < 0) {
> cx231xx_release_resources(dev);
> + return retval;
> + }
> +
> + retval = media_device_register(dev->media_dev);
> + if (retval < 0) {
> + dev_err(dev->dev, "Couldn't register media device. Error: %d\n",
> + retval);
> + cx231xx_release_resources(dev);
> }
>
> return retval;
> +
> err_video_alt:
> /* cx231xx_uninit_dev: */
> cx231xx_close_extension(dev);
> diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> index 6d3f61f6dc77..13a6e5a238db 100644
> --- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> +++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
> @@ -400,7 +400,7 @@ skip_feed_stop:
> return ret;
> }
>
> -static void dvb_usbv2_media_device_register(struct dvb_usb_adapter *adap)
> +static void dvb_usbv2_media_device_init(struct dvb_usb_adapter *adap)
> {
> #ifdef CONFIG_MEDIA_CONTROLLER_DVB
> struct media_device *mdev;
> @@ -420,7 +420,7 @@ static void dvb_usbv2_media_device_register(struct dvb_usb_adapter *adap)
> mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
> mdev->driver_version = LINUX_VERSION_CODE;
>
> - ret = media_device_register(mdev);
> + ret = media_device_init(mdev);
> if (ret) {
> dev_err(&d->udev->dev,
> "Couldn't create a media device. Error: %d\n",
> @@ -444,6 +444,7 @@ static void dvb_usbv2_media_device_unregister(struct dvb_usb_adapter *adap)
> return;
>
> media_device_unregister(adap->dvb_adap.mdev);
> + media_device_cleanup(adap->dvb_adap.mdev);
> kfree(adap->dvb_adap.mdev);
> adap->dvb_adap.mdev = NULL;
>
> @@ -467,7 +468,7 @@ static int dvb_usbv2_adapter_dvb_init(struct dvb_usb_adapter *adap)
>
> adap->dvb_adap.priv = adap;
>
> - dvb_usbv2_media_device_register(adap);
> + dvb_usbv2_media_device_init(adap);
>
> if (d->props->read_mac_address) {
> ret = d->props->read_mac_address(adap,
> @@ -702,6 +703,10 @@ static int dvb_usbv2_adapter_frontend_init(struct dvb_usb_adapter *adap)
> if (ret < 0)
> goto err_dvb_unregister_frontend;
>
> + ret = media_device_register(adap->dvb_adap.mdev);
> + if (ret)
> + goto err_dvb_unregister_frontend;
> +
> return 0;
>
> err_dvb_unregister_frontend:
> diff --git a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
> index b51dbdf03f42..b975fc1ea798 100644
> --- a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
> +++ b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c
> @@ -95,7 +95,7 @@ static int dvb_usb_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
> return dvb_usb_ctrl_feed(dvbdmxfeed, 0);
> }
>
> -static void dvb_usb_media_device_register(struct dvb_usb_adapter *adap)
> +static void dvb_usb_media_device_init(struct dvb_usb_adapter *adap)
> {
> #ifdef CONFIG_MEDIA_CONTROLLER_DVB
> struct media_device *mdev;
> @@ -115,7 +115,7 @@ static void dvb_usb_media_device_register(struct dvb_usb_adapter *adap)
> mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
> mdev->driver_version = LINUX_VERSION_CODE;
>
> - ret = media_device_register(mdev);
> + ret = media_device_init(mdev);
> if (ret) {
> dev_err(&d->udev->dev,
> "Couldn't create a media device. Error: %d\n",
> @@ -136,6 +136,7 @@ static void dvb_usb_media_device_unregister(struct dvb_usb_adapter *adap)
> return;
>
> media_device_unregister(adap->dvb_adap.mdev);
> + media_device_cleanup(adap->dvb_adap.mdev);
> kfree(adap->dvb_adap.mdev);
> adap->dvb_adap.mdev = NULL;
> #endif
> @@ -154,7 +155,7 @@ int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, short *adapter_nums)
> }
> adap->dvb_adap.priv = adap;
>
> - dvb_usb_media_device_register(adap);
> + dvb_usb_media_device_init(adap);
>
> if (adap->dev->props.read_mac_address) {
> if (adap->dev->props.read_mac_address(adap->dev, adap->dvb_adap.proposed_mac) == 0)
> @@ -322,6 +323,12 @@ int dvb_usb_adapter_frontend_init(struct dvb_usb_adapter *adap)
> return ret;
>
> ret = dvb_create_media_graph(&adap->dvb_adap);
> + if (ret < 0)
> + return ret;
> +
> + ret = media_device_register(adap->dvb_adap.mdev);
> + if (ret < 0)
> + return ret;
>
> return ret;
> }
> diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
> index c945e4c2fbd4..a451793d7c46 100644
> --- a/drivers/media/usb/siano/smsusb.c
> +++ b/drivers/media/usb/siano/smsusb.c
> @@ -361,17 +361,27 @@ static void *siano_media_device_register(struct smsusb_device_t *dev,
> mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
> mdev->driver_version = LINUX_VERSION_CODE;
>
> + ret = media_device_init(mdev);
> + if (ret < 0) {
> + pr_err("Couldn't initialize a media device. Error: %d\n",
> + ret);
> + goto err;
> + }
> +
> ret = media_device_register(mdev);
> if (ret) {
> pr_err("Couldn't create a media device. Error: %d\n",
> ret);
> - kfree(mdev);
> - return NULL;
> + media_device_cleanup(mdev);
> + goto err;
> }
>
> pr_info("media controller created\n");
>
> return mdev;
> +err:
> + kfree(mdev);
> + return NULL;
> #else
> return NULL;
> #endif
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 4b5b3e8fb7d3..27d111102626 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1655,6 +1655,7 @@ static void uvc_delete(struct uvc_device *dev)
> #ifdef CONFIG_MEDIA_CONTROLLER
> if (media_devnode_is_registered(&dev->mdev.devnode))
> media_device_unregister(&dev->mdev);
> + media_device_cleanup(&dev->mdev);
> #endif
>
> list_for_each_safe(p, n, &dev->chains) {
> @@ -1905,7 +1906,7 @@ static int uvc_probe(struct usb_interface *intf,
> "linux-uvc-devel mailing list.\n");
> }
>
> - /* Register the media and V4L2 devices. */
> + /* Initialize the media device and register the V4L2 device. */
> #ifdef CONFIG_MEDIA_CONTROLLER
> dev->mdev.dev = &intf->dev;
> strlcpy(dev->mdev.model, dev->name, sizeof(dev->mdev.model));
> @@ -1915,7 +1916,7 @@ static int uvc_probe(struct usb_interface *intf,
> strcpy(dev->mdev.bus_info, udev->devpath);
> dev->mdev.hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
> dev->mdev.driver_version = LINUX_VERSION_CODE;
> - if (media_device_register(&dev->mdev) < 0)
> + if (media_device_init(&dev->mdev) < 0)
> goto error;
>
> dev->vdev.mdev = &dev->mdev;
> @@ -1935,6 +1936,10 @@ static int uvc_probe(struct usb_interface *intf,
> if (uvc_register_chains(dev) < 0)
> goto error;
>
> + /* Register the media device node */
> + if (media_device_register(&dev->mdev) < 0)
> + goto error;
> +
> /* Save our data pointer in the interface data. */
> usb_set_intfdata(intf, dev);
>
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 1b12774a9ab4..8d5d9881404d 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -106,6 +106,8 @@ struct media_device {
> /* media_devnode to media_device */
> #define to_media_device(node) container_of(node, struct media_device, devnode)
>
> +int __must_check media_device_init(struct media_device *mdev);
> +void media_device_cleanup(struct media_device *mdev);
> int __must_check __media_device_register(struct media_device *mdev,
> struct module *owner);
> #define media_device_register(mdev) __media_device_register(mdev, THIS_MODULE)
>


--
Kind regards,

Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/