From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751939AbdBBWpl (ORCPT ); Thu, 2 Feb 2017 17:45:41 -0500 Received: from pandora.armlinux.org.uk ([78.32.30.218]:42742 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751561AbdBBWpk (ORCPT ); Thu, 2 Feb 2017 17:45:40 -0500 Date: Thu, 2 Feb 2017 22:44:53 +0000 From: Russell King - ARM Linux To: Steve Longerbeam Cc: robh+dt@kernel.org, mark.rutland@arm.com, shawnguo@kernel.org, kernel@pengutronix.de, fabio.estevam@nxp.com, mchehab@kernel.org, hverkuil@xs4all.nl, nick@shmanahar.org, markus.heiser@darmarIT.de, p.zabel@pengutronix.de, laurent.pinchart+renesas@ideasonboard.com, bparrot@ti.com, geert@linux-m68k.org, arnd@arndb.de, sudipm.mukherjee@gmail.com, minghsiu.tsai@mediatek.com, tiffany.lin@mediatek.com, jean-christophe.trotin@st.com, horms+renesas@verge.net.au, niklas.soderlund+renesas@ragnatech.se, robert.jarzmik@free.fr, songjun.wu@microchip.com, andrew-ct.chen@mediatek.com, gregkh@linuxfoundation.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org, devel@driverdev.osuosl.org, Steve Longerbeam Subject: Re: [PATCH v3 16/24] media: Add i.MX media core driver Message-ID: <20170202224453.GY27312@n2100.armlinux.org.uk> References: <1483755102-24785-1-git-send-email-steve_longerbeam@mentor.com> <1483755102-24785-17-git-send-email-steve_longerbeam@mentor.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1483755102-24785-17-git-send-email-steve_longerbeam@mentor.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 06, 2017 at 06:11:34PM -0800, Steve Longerbeam wrote: > +struct imx_media_dev { > + struct media_device md; > + struct v4l2_device v4l2_dev; This is similarly buggy. struct v4l2_device { struct device *dev; #if defined(CONFIG_MEDIA_CONTROLLER) struct media_device *mdev; #endif struct list_head subdevs; spinlock_t lock; char name[V4L2_DEVICE_NAME_SIZE]; void (*notify)(struct v4l2_subdev *sd, unsigned int notification, void *arg); struct v4l2_ctrl_handler *ctrl_handler; struct v4l2_prio_state prio; struct kref ref; void (*release)(struct v4l2_device *v4l2_dev); }; Notice the kref and release function. This is the only way the memory backing "struct v4l2_device" may be released. If you wish to embed this structure into another structure, then the lifetime of that other structure is determined by this one. IOW, when this release function is called, only then may you kfree() the memory backing struct imx_media_dev. > + struct device *dev; And... do you need all these struct device pointers? imxmd->dev = dev; imxmd->md.dev = dev; As media_device already contains a pointer, can't you re-use that? -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux Subject: Re: [PATCH v3 16/24] media: Add i.MX media core driver Date: Thu, 2 Feb 2017 22:44:53 +0000 Message-ID: <20170202224453.GY27312@n2100.armlinux.org.uk> References: <1483755102-24785-1-git-send-email-steve_longerbeam@mentor.com> <1483755102-24785-17-git-send-email-steve_longerbeam@mentor.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1483755102-24785-17-git-send-email-steve_longerbeam-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Steve Longerbeam Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, mark.rutland-5wv7dgnIgG8@public.gmane.org, shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, fabio.estevam-3arQi8VN3Tc@public.gmane.org, mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org, nick-gcszYUEDH4VrovVCs/uTlw@public.gmane.org, markus.heiser-O6JHGLzbNUwb1SvskN2V4Q@public.gmane.org, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org, bparrot-l0cyMroinI0@public.gmane.org, geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org, arnd-r2nGTMty4D4@public.gmane.org, sudipm.mukherjee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org, tiffany.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org, jean-christophe.trotin-qxv4g6HH51o@public.gmane.org, horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org, niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw@public.gmane.org, robert.jarzmik-GANU6spQydw@public.gmane.org, songjun.wu-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org, andrew-ct.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b@public.gmane.org, Steve Longerbeam List-Id: devicetree@vger.kernel.org On Fri, Jan 06, 2017 at 06:11:34PM -0800, Steve Longerbeam wrote: > +struct imx_media_dev { > + struct media_device md; > + struct v4l2_device v4l2_dev; This is similarly buggy. struct v4l2_device { struct device *dev; #if defined(CONFIG_MEDIA_CONTROLLER) struct media_device *mdev; #endif struct list_head subdevs; spinlock_t lock; char name[V4L2_DEVICE_NAME_SIZE]; void (*notify)(struct v4l2_subdev *sd, unsigned int notification, void *arg); struct v4l2_ctrl_handler *ctrl_handler; struct v4l2_prio_state prio; struct kref ref; void (*release)(struct v4l2_device *v4l2_dev); }; Notice the kref and release function. This is the only way the memory backing "struct v4l2_device" may be released. If you wish to embed this structure into another structure, then the lifetime of that other structure is determined by this one. IOW, when this release function is called, only then may you kfree() the memory backing struct imx_media_dev. > + struct device *dev; And... do you need all these struct device pointers? imxmd->dev = dev; imxmd->md.dev = dev; As media_device already contains a pointer, can't you re-use that? -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: linux@armlinux.org.uk (Russell King - ARM Linux) Date: Thu, 2 Feb 2017 22:44:53 +0000 Subject: [PATCH v3 16/24] media: Add i.MX media core driver In-Reply-To: <1483755102-24785-17-git-send-email-steve_longerbeam@mentor.com> References: <1483755102-24785-1-git-send-email-steve_longerbeam@mentor.com> <1483755102-24785-17-git-send-email-steve_longerbeam@mentor.com> Message-ID: <20170202224453.GY27312@n2100.armlinux.org.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, Jan 06, 2017 at 06:11:34PM -0800, Steve Longerbeam wrote: > +struct imx_media_dev { > + struct media_device md; > + struct v4l2_device v4l2_dev; This is similarly buggy. struct v4l2_device { struct device *dev; #if defined(CONFIG_MEDIA_CONTROLLER) struct media_device *mdev; #endif struct list_head subdevs; spinlock_t lock; char name[V4L2_DEVICE_NAME_SIZE]; void (*notify)(struct v4l2_subdev *sd, unsigned int notification, void *arg); struct v4l2_ctrl_handler *ctrl_handler; struct v4l2_prio_state prio; struct kref ref; void (*release)(struct v4l2_device *v4l2_dev); }; Notice the kref and release function. This is the only way the memory backing "struct v4l2_device" may be released. If you wish to embed this structure into another structure, then the lifetime of that other structure is determined by this one. IOW, when this release function is called, only then may you kfree() the memory backing struct imx_media_dev. > + struct device *dev; And... do you need all these struct device pointers? imxmd->dev = dev; imxmd->md.dev = dev; As media_device already contains a pointer, can't you re-use that? -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.