From patchwork Tue Dec 15 12:22:46 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mauro Carvalho Chehab X-Patchwork-Id: 7854201 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 0136C9F349 for ; Tue, 15 Dec 2015 12:25:38 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 325EF202A1 for ; Tue, 15 Dec 2015 12:25:36 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id ABAF12034C for ; Tue, 15 Dec 2015 12:25:34 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1a8odo-0000Xn-6X; Tue, 15 Dec 2015 12:23:40 +0000 Received: from [191.33.142.179] (helo=smtp.w2.samsung.com) by bombadil.infradead.org with esmtpsa (Exim 4.80.1 #2 (Red Hat Linux)) id 1a8odK-0000Lz-5Q; Tue, 15 Dec 2015 12:23:10 +0000 Received: from mchehab by smtp.w2.samsung.com with local (Exim 4.86) (envelope-from ) id 1a8od2-0005Oy-TV; Tue, 15 Dec 2015 10:22:52 -0200 From: Mauro Carvalho Chehab To: Subject: [PATCH v2] [media] media-device: handle errors at media_device_init() Date: Tue, 15 Dec 2015 10:22:46 -0200 Message-Id: X-Mailer: git-send-email 2.5.0 X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Krzysztof Kozlowski , linux-sh@vger.kernel.org, =?UTF-8?q?Rafael=20Louren=C3=A7o=20de=20Lima=20Chehab?= , Laurent Pinchart , Sylwester Nawrocki , Matthias Schwarzott , Olli Salonen , Alexey Khoroshilov , linux-samsung-soc@vger.kernel.org, Michal Simek , Javier Martinez Canillas , Antti Palosaari , Hans Verkuil , =?UTF-8?q?S=C3=B6ren=20Brinkmann?= , Luis de Bethencourt , Linux Media Mailing List , Mauro Carvalho Chehab , Mauro Carvalho Chehab , linux-arm-kernel@lists.infradead.org, Tommi Rantala , Hyun Kwon , Kyungmin Park , Kukjin Kim , Sakari Ailus MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Changeset 43ac4401dca9 ("[media] media-device: split media initialization and registration") broke media device register into two separate functions, but introduced a BUG_ON() and made media_device_init() void. It also introduced several warnings. Instead of adding BUG_ON(), let's revert to WARN_ON() and fix the init code in a way that, if something goes wrong during device init, driver probe will fail without causing the Kernel to BUG. Signed-off-by: Mauro Carvalho Chehab Reviewed-by: Javier Martinez Canillas Tested-by: Javier Martinez Canillas --- drivers/media/media-device.c | 8 ++++--- drivers/media/platform/exynos4-is/media-dev.c | 7 +++++- drivers/media/platform/omap3isp/isp.c | 9 ++++++-- drivers/media/platform/s3c-camif/camif-core.c | 4 +++- drivers/media/platform/vsp1/vsp1_drv.c | 7 +++++- drivers/media/platform/xilinx/xilinx-vipp.c | 7 +++++- drivers/media/usb/au0828/au0828-core.c | 26 +++++++++++++++++---- drivers/media/usb/cx231xx/cx231xx-cards.c | 23 +++++++++++++++---- drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 33 +++++++++++++++++++-------- drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 32 +++++++++++++++++++------- drivers/media/usb/siano/smsusb.c | 6 ++++- drivers/media/usb/uvc/uvc_driver.c | 3 ++- include/media/media-device.h | 2 +- 13 files changed, 128 insertions(+), 39 deletions(-) diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c index 76e2b2f3a15f..34621605eed7 100644 --- a/drivers/media/media-device.c +++ b/drivers/media/media-device.c @@ -625,10 +625,10 @@ EXPORT_SYMBOL_GPL(media_device_unregister_entity); * - dev must point to the parent device * - model must be filled with the device model name */ -void media_device_init(struct media_device *mdev) +int __must_check media_device_init(struct media_device *mdev) { - - BUG_ON(mdev->dev == NULL); + if (WARN_ON(mdev->dev == NULL)) + return -EINVAL; INIT_LIST_HEAD(&mdev->entities); INIT_LIST_HEAD(&mdev->interfaces); @@ -638,6 +638,8 @@ void media_device_init(struct media_device *mdev) mutex_init(&mdev->graph_mutex); dev_dbg(mdev->dev, "Media device initialized\n"); + + return 0; } EXPORT_SYMBOL_GPL(media_device_init); diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c index 27663dd45294..4fea0037dade 100644 --- a/drivers/media/platform/exynos4-is/media-dev.c +++ b/drivers/media/platform/exynos4-is/media-dev.c @@ -1353,7 +1353,11 @@ static int fimc_md_probe(struct platform_device *pdev) return ret; } - media_device_init(&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); + goto err_v4l2_dev; + } ret = fimc_md_get_clocks(fmd); if (ret) @@ -1424,6 +1428,7 @@ err_m_ent: fimc_md_unregister_entities(fmd); err_md: media_device_cleanup(&fmd->media_dev); +err_v4l2_dev: v4l2_device_unregister(&fmd->v4l2_dev); return ret; } diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index 942b189c0eca..347bfdd8ce65 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -1876,7 +1876,12 @@ 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; - media_device_init(&isp->media_dev); + ret = media_device_init(&isp->media_dev); + if (ret < 0) { + dev_err(isp->dev, "%s: Media device registration failed (%d)\n", + __func__, ret); + return ret; + } isp->v4l2_dev.mdev = &isp->media_dev; ret = v4l2_device_register(isp->dev, &isp->v4l2_dev); @@ -2361,7 +2366,7 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async) } } - ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev); + ret = v4l2_device_register_subdev_nodes(v4l2_dev); if (ret < 0) return ret; diff --git a/drivers/media/platform/s3c-camif/camif-core.c b/drivers/media/platform/s3c-camif/camif-core.c index ea02b7ef2119..6e6ad152adbc 100644 --- a/drivers/media/platform/s3c-camif/camif-core.c +++ b/drivers/media/platform/s3c-camif/camif-core.c @@ -328,7 +328,9 @@ static int camif_media_dev_init(struct camif_dev *camif) if (ret < 0) return ret; - media_device_init(md); + ret = media_device_init(md); + if (ret < 0) + v4l2_device_unregister(v4l2_dev); return ret; } diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c index 42dff9d020af..1e319eaeb8fe 100644 --- a/drivers/media/platform/vsp1/vsp1_drv.c +++ b/drivers/media/platform/vsp1/vsp1_drv.c @@ -142,7 +142,12 @@ 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)); - media_device_init(mdev); + ret = media_device_init(mdev); + if (ret < 0) { + dev_err(vsp1->dev, "media device registration failed (%d)\n", + ret); + return ret; + } vdev->mdev = mdev; ret = v4l2_device_register(vsp1->dev, vdev); diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c index e795a4501e8b..7c5d23010d6f 100644 --- a/drivers/media/platform/xilinx/xilinx-vipp.c +++ b/drivers/media/platform/xilinx/xilinx-vipp.c @@ -585,7 +585,12 @@ static int xvip_composite_v4l2_init(struct xvip_composite_device *xdev) sizeof(xdev->media_dev.model)); xdev->media_dev.hw_revision = 0; - media_device_init(&xdev->media_dev); + ret = media_device_init(&xdev->media_dev); + if (ret < 0) { + dev_err(xdev->dev, "media device registration failed (%d)\n", + ret); + return ret; + } xdev->v4l2_dev.mdev = &xdev->media_dev; ret = v4l2_device_register(xdev->dev, &xdev->v4l2_dev); diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c index e7aab47ae4f5..2f91bbc633b4 100644 --- a/drivers/media/usb/au0828/au0828-core.c +++ b/drivers/media/usb/au0828/au0828-core.c @@ -217,15 +217,16 @@ static void au0828_usb_disconnect(struct usb_interface *interface) au0828_usb_release(dev); } -static void au0828_media_device_init(struct au0828_dev *dev, - struct usb_device *udev) +static int au0828_media_device_init(struct au0828_dev *dev, + struct usb_device *udev) { #ifdef CONFIG_MEDIA_CONTROLLER struct media_device *mdev; + int ret; mdev = kzalloc(sizeof(*mdev), GFP_KERNEL); if (!mdev) - return; + return -ENOMEM; mdev->dev = &udev->dev; @@ -239,10 +240,18 @@ static void au0828_media_device_init(struct au0828_dev *dev, mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice); mdev->driver_version = LINUX_VERSION_CODE; - media_device_init(mdev); + ret = media_device_init(mdev); + if (ret) { + pr_err( + "Couldn't create a media device. Error: %d\n", + ret); + kfree(mdev); + return ret; + } dev->media_dev = mdev; #endif + return 0; } @@ -368,7 +377,14 @@ static int au0828_usb_probe(struct usb_interface *interface, dev->board = au0828_boards[dev->boardnr]; /* Initialize the media controller */ - au0828_media_device_init(dev, usbdev); + retval = au0828_media_device_init(dev, usbdev); + if (retval) { + pr_err("%s() au0828_media_device_init failed\n", + __func__); + mutex_unlock(&dev->lock); + kfree(dev); + return retval; + } #ifdef CONFIG_VIDEO_AU0828_V4L2 dev->v4l2_dev.release = au0828_usb_v4l2_release; diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c index 2e997f09a4cd..35692d19b652 100644 --- a/drivers/media/usb/cx231xx/cx231xx-cards.c +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c @@ -1206,15 +1206,16 @@ void cx231xx_release_resources(struct cx231xx *dev) clear_bit(dev->devno, &cx231xx_devused); } -static void cx231xx_media_device_init(struct cx231xx *dev, - struct usb_device *udev) +static int cx231xx_media_device_init(struct cx231xx *dev, + struct usb_device *udev) { #ifdef CONFIG_MEDIA_CONTROLLER struct media_device *mdev; + int ret; mdev = kzalloc(sizeof(*mdev), GFP_KERNEL); if (!mdev) - return; + return -ENOMEM; mdev->dev = dev->dev; strlcpy(mdev->model, dev->board.name, sizeof(mdev->model)); @@ -1224,10 +1225,18 @@ static void cx231xx_media_device_init(struct cx231xx *dev, mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice); mdev->driver_version = LINUX_VERSION_CODE; - media_device_init(mdev); + ret = media_device_init(mdev); + if (ret) { + dev_err(dev->dev, + "Couldn't create a media device. Error: %d\n", + ret); + kfree(mdev); + return ret; + } dev->media_dev = mdev; #endif + return 0; } static int cx231xx_create_media_graph(struct cx231xx *dev) @@ -1663,7 +1672,11 @@ static int cx231xx_usb_probe(struct usb_interface *interface, usb_set_intfdata(interface, dev); /* Initialize the media controller */ - cx231xx_media_device_init(dev, udev); + retval = cx231xx_media_device_init(dev, udev); + if (retval) { + dev_err(d, "cx231xx_media_device_init failed\n"); + goto err_v4l2; + } /* Create v4l2 device */ #ifdef CONFIG_MEDIA_CONTROLLER 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 a0413dcc3361..9992ac6e63cc 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_init(struct dvb_usb_adapter *adap) +static int dvb_usbv2_media_device_init(struct dvb_usb_adapter *adap) { #ifdef CONFIG_MEDIA_CONTROLLER_DVB struct media_device *mdev; @@ -410,7 +410,7 @@ static void dvb_usbv2_media_device_init(struct dvb_usb_adapter *adap) mdev = kzalloc(sizeof(*mdev), GFP_KERNEL); if (!mdev) - return; + return -ENOMEM; mdev->dev = &udev->dev; strlcpy(mdev->model, d->name, sizeof(mdev->model)); @@ -420,19 +420,28 @@ static void dvb_usbv2_media_device_init(struct dvb_usb_adapter *adap) mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice); mdev->driver_version = LINUX_VERSION_CODE; - media_device_init(mdev); + ret = media_device_init(mdev); + if (ret) { + dev_err(&d->udev->dev, + "Couldn't create a media device. Error: %d\n", + ret); + kfree(mdev); + return ret; + } dvb_register_media_controller(&adap->dvb_adap, mdev); dev_info(&d->udev->dev, "media controller created\n"); - #endif + return 0; } -static void dvb_usbv2_media_device_register(struct dvb_usb_adapter *adap) +static int dvb_usbv2_media_device_register(struct dvb_usb_adapter *adap) { #ifdef CONFIG_MEDIA_CONTROLLER_DVB - media_device_register(adap->dvb_adap.mdev); + return media_device_register(adap->dvb_adap.mdev); +#else + return 0; #endif } @@ -468,7 +477,12 @@ static int dvb_usbv2_adapter_dvb_init(struct dvb_usb_adapter *adap) adap->dvb_adap.priv = adap; - dvb_usbv2_media_device_init(adap); + ret = dvb_usbv2_media_device_init(adap); + if (ret < 0) { + dev_dbg(&d->udev->dev, "%s: dvb_usbv2_media_device_init() failed=%d\n", + __func__, ret); + goto err_dvb_register_mc; + } if (d->props->read_mac_address) { ret = d->props->read_mac_address(adap, @@ -519,6 +533,7 @@ err_dvb_dmxdev_init: dvb_dmx_release(&adap->demux); err_dvb_dmx_init: dvb_usbv2_media_device_unregister(adap); +err_dvb_register_mc: dvb_unregister_adapter(&adap->dvb_adap); err_dvb_register_adapter: adap->dvb_adap.priv = NULL; @@ -703,9 +718,9 @@ static int dvb_usbv2_adapter_frontend_init(struct dvb_usb_adapter *adap) if (ret < 0) goto err_dvb_unregister_frontend; - dvb_usbv2_media_device_register(&adap->dvb_adap); + ret = dvb_usbv2_media_device_register(adap); - return 0; + return ret; err_dvb_unregister_frontend: for (i = count_registered - 1; i >= 0; i--) diff --git a/drivers/media/usb/dvb-usb/dvb-usb-dvb.c b/drivers/media/usb/dvb-usb/dvb-usb-dvb.c index 4086d9626664..920396f935ab 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_init(struct dvb_usb_adapter *adap) +static int dvb_usb_media_device_init(struct dvb_usb_adapter *adap) { #ifdef CONFIG_MEDIA_CONTROLLER_DVB struct media_device *mdev; @@ -105,7 +105,7 @@ static void dvb_usb_media_device_init(struct dvb_usb_adapter *adap) mdev = kzalloc(sizeof(*mdev), GFP_KERNEL); if (!mdev) - return; + return -EINVAL; mdev->dev = &udev->dev; strlcpy(mdev->model, d->desc->name, sizeof(mdev->model)); @@ -115,18 +115,27 @@ static void dvb_usb_media_device_init(struct dvb_usb_adapter *adap) mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice); mdev->driver_version = LINUX_VERSION_CODE; - media_device_init(mdev); - + ret = media_device_init(mdev); + if (ret) { + dev_err(&d->udev->dev, + "Couldn't create a media device. Error: %d\n", + ret); + kfree(mdev); + return ret; + } dvb_register_media_controller(&adap->dvb_adap, mdev); dev_info(&d->udev->dev, "media controller created\n"); #endif + return 0; } -static void dvb_usb_media_device_register(struct dvb_usb_adapter *adap) +static int dvb_usb_media_device_register(struct dvb_usb_adapter *adap) { #ifdef CONFIG_MEDIA_CONTROLLER_DVB - media_device_register(adap->dvb_adap.mdev); + return media_device_register(adap->dvb_adap.mdev); +#else + return 0; #endif } @@ -156,7 +165,11 @@ int dvb_usb_adapter_dvb_init(struct dvb_usb_adapter *adap, short *adapter_nums) } adap->dvb_adap.priv = adap; - dvb_usb_media_device_init(adap); + ret = dvb_usb_media_device_init(adap); + if (ret < 0) { + deb_info("dvb_usb_media_device_init failed: error %d", ret); + goto err_mc; + } if (adap->dev->props.read_mac_address) { if (adap->dev->props.read_mac_address(adap->dev, adap->dvb_adap.proposed_mac) == 0) @@ -206,6 +219,7 @@ err_dmx_dev: dvb_dmx_release(&adap->demux); err_dmx: dvb_usb_media_device_unregister(adap); +err_mc: dvb_unregister_adapter(&adap->dvb_adap); err: return ret; @@ -324,8 +338,10 @@ int dvb_usb_adapter_frontend_init(struct dvb_usb_adapter *adap) return ret; ret = dvb_create_media_graph(&adap->dvb_adap); + if (ret) + return ret; - dvb_usb_media_device_register(adap); + ret = dvb_usb_media_device_register(adap); return ret; } diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c index 8abbd3cc8eba..327180efa264 100644 --- a/drivers/media/usb/siano/smsusb.c +++ b/drivers/media/usb/siano/smsusb.c @@ -361,7 +361,11 @@ 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; - media_device_init(mdev); + ret = media_device_init(mdev); + if (ret) { + kfree(mdev); + return NULL; + } ret = media_device_register(mdev); if (ret) { diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 96fc2c2efa7b..4dfd3eb814e7 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1917,7 +1917,8 @@ 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; - media_device_init(&dev->mdev); + if (media_device_init(&dev->mdev) < 0) + goto error; dev->vdev.mdev = &dev->mdev; #endif diff --git a/include/media/media-device.h b/include/media/media-device.h index 572190860cbb..f09be5d47a7d 100644 --- a/include/media/media-device.h +++ b/include/media/media-device.h @@ -351,7 +351,7 @@ struct media_device { * within the media device, create pad to pad links and then finally register * the media device by calling media_device_register() as a final step. */ -void media_device_init(struct media_device *mdev); +int __must_check media_device_init(struct media_device *mdev); /** * media_device_cleanup() - Cleanups a media device element