Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755600AbcKKKwK (ORCPT ); Fri, 11 Nov 2016 05:52:10 -0500 Received: from lb1-smtp-cloud2.xs4all.net ([194.109.24.21]:43591 "EHLO lb1-smtp-cloud2.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750979AbcKKKwI (ORCPT ); Fri, 11 Nov 2016 05:52:08 -0500 Subject: Re: [PATCH] [media] mtk-mdp: allocate video_device dynamically To: Minghsiu Tsai , Hans Verkuil , daniel.thompson@linaro.org, Rob Herring , Mauro Carvalho Chehab , Matthias Brugger , Daniel Kurtz , Pawel Osciak References: <1478522529-57129-1-git-send-email-minghsiu.tsai@mediatek.com> <1478522529-57129-2-git-send-email-minghsiu.tsai@mediatek.com> Cc: srv_heupstream@mediatek.com, Eddie Huang , Yingjoe Chen , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org, linux-mediatek@lists.infradead.org From: Hans Verkuil Message-ID: Date: Fri, 11 Nov 2016 11:51:57 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1478522529-57129-2-git-send-email-minghsiu.tsai@mediatek.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3607 Lines: 102 Almost correct: On 11/07/2016 01:42 PM, Minghsiu Tsai wrote: > It can fix known problems with embedded video_device structs. > > Signed-off-by: Minghsiu Tsai > --- > drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 2 +- > drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 33 ++++++++++++++++----------- > 2 files changed, 21 insertions(+), 14 deletions(-) > > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h > index 848569d..ad1cff3 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h > @@ -167,7 +167,7 @@ struct mtk_mdp_dev { > struct mtk_mdp_comp *comp[MTK_MDP_COMP_ID_MAX]; > struct v4l2_m2m_dev *m2m_dev; > struct list_head ctx_list; > - struct video_device vdev; > + struct video_device *vdev; > struct v4l2_device v4l2_dev; > struct workqueue_struct *job_wq; > struct platform_device *vpu_dev; > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > index 9a747e7..b8dee1c 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c > @@ -1236,16 +1236,22 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp) > int ret; > > mdp->variant = &mtk_mdp_default_variant; > - mdp->vdev.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; > - mdp->vdev.fops = &mtk_mdp_m2m_fops; > - mdp->vdev.ioctl_ops = &mtk_mdp_m2m_ioctl_ops; > - mdp->vdev.release = video_device_release_empty; > - mdp->vdev.lock = &mdp->lock; > - mdp->vdev.vfl_dir = VFL_DIR_M2M; > - mdp->vdev.v4l2_dev = &mdp->v4l2_dev; > - snprintf(mdp->vdev.name, sizeof(mdp->vdev.name), "%s:m2m", > + mdp->vdev = video_device_alloc(); > + if (!mdp->vdev) { > + dev_err(dev, "failed to allocate video device\n"); > + ret = -ENOMEM; > + goto err_video_alloc; > + } > + mdp->vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING; > + mdp->vdev->fops = &mtk_mdp_m2m_fops; > + mdp->vdev->ioctl_ops = &mtk_mdp_m2m_ioctl_ops; > + mdp->vdev->release = video_device_release; > + mdp->vdev->lock = &mdp->lock; > + mdp->vdev->vfl_dir = VFL_DIR_M2M; > + mdp->vdev->v4l2_dev = &mdp->v4l2_dev; > + snprintf(mdp->vdev->name, sizeof(mdp->vdev->name), "%s:m2m", > MTK_MDP_MODULE_NAME); > - video_set_drvdata(&mdp->vdev, mdp); > + video_set_drvdata(mdp->vdev, mdp); > > mdp->m2m_dev = v4l2_m2m_init(&mtk_mdp_m2m_ops); > if (IS_ERR(mdp->m2m_dev)) { > @@ -1254,26 +1260,27 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp) > goto err_m2m_init; > } > > - ret = video_register_device(&mdp->vdev, VFL_TYPE_GRABBER, 2); > + ret = video_register_device(mdp->vdev, VFL_TYPE_GRABBER, 2); > if (ret) { > dev_err(dev, "failed to register video device\n"); > goto err_vdev_register; > } > > v4l2_info(&mdp->v4l2_dev, "driver registered as /dev/video%d", > - mdp->vdev.num); > + mdp->vdev->num); > return 0; > > err_vdev_register: > v4l2_m2m_release(mdp->m2m_dev); > err_m2m_init: > - video_device_release(&mdp->vdev); > + video_unregister_device(mdp->vdev); This should remain video_device_release: the video_register_device call failed, so the device hasn't been registered. In that case just release the device (i.e. free the allocated memory). > +err_video_alloc: > > return ret; > } > > void mtk_mdp_unregister_m2m_device(struct mtk_mdp_dev *mdp) > { > - video_device_release(&mdp->vdev); > + video_unregister_device(mdp->vdev); > v4l2_m2m_release(mdp->m2m_dev); > } > Regards, Hans