Message ID | 1368692074-483-2-git-send-email-a.hajda@samsung.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Laurent Pinchart |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1UctLI-0003Sc-1R; Thu, 16 May 2013 10:15:16 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.72/mailfrontend-6) with esmtp id 1UctLG-0002NN-3H; Thu, 16 May 2013 10:15:15 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756113Ab3EPIPJ (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Thu, 16 May 2013 04:15:09 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:32066 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755941Ab3EPIPD (ORCPT <rfc822;linux-media@vger.kernel.org>); Thu, 16 May 2013 04:15:03 -0400 Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MMV00189TKYHT40@mailout4.w1.samsung.com> for linux-media@vger.kernel.org; Thu, 16 May 2013 09:15:01 +0100 (BST) X-AuditID: cbfec7f5-b7f376d000001ec6-86-51949584b3b5 Received: from eusync2.samsung.com ( [203.254.199.212]) by eucpsbgm2.samsung.com (EUCPMTA) with SMTP id 8C.1D.07878.48594915; Thu, 16 May 2013 09:15:00 +0100 (BST) Received: from AMDC1061.digital.local ([106.116.147.88]) by eusync2.samsung.com (Oracle Communications Messaging Server 7u4-23.01 (7.0.4.23.0) 64bit (built Aug 10 2011)) with ESMTPA id <0MMV00JNCTKOHX00@eusync2.samsung.com>; Thu, 16 May 2013 09:15:00 +0100 (BST) From: Andrzej Hajda <a.hajda@samsung.com> To: linux-media@vger.kernel.org Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Sylwester Nawrocki <s.nawrocki@samsung.com>, Sakari Ailus <sakari.ailus@iki.fi>, Kyungmin Park <kyungmin.park@samsung.com>, hj210.choi@samsung.com, sw0312.kim@samsung.com, Andrzej Hajda <a.hajda@samsung.com> Subject: [PATCH RFC v3 1/3] media: added managed media entity initialization Date: Thu, 16 May 2013 10:14:32 +0200 Message-id: <1368692074-483-2-git-send-email-a.hajda@samsung.com> X-Mailer: git-send-email 1.7.10.4 In-reply-to: <1368692074-483-1-git-send-email-a.hajda@samsung.com> References: <1368692074-483-1-git-send-email-a.hajda@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrGJMWRmVeSWpSXmKPExsVy+t/xK7otU6cEGrz9JGdxa905VovHG68x W5xtesNu0TlxCbtFz4atrBaH37SzWpzZv5LNYsbkl2wOHB6zO2ayehz+upDFo2/LKkaPz5vk AliiuGxSUnMyy1KL9O0SuDKmtpsUdIlXLD65i7GBcalwFyMnh4SAicSEmRtZIGwxiQv31rN1 MXJxCAksZZR4PWExM4TTxySxff5FsCo2AU2Jv5tvsoHYIgLyEk96b4B1MAtMZJK4+ncZWEJY wEdiwfn5TCA2i4CqxOJtS1lBbF4BR4m9Ez8xQ6xTlOh+NgGsnlPASWL18kdgthBQzdcbvYwT GHkXMDKsYhRNLU0uKE5KzzXSK07MLS7NS9dLzs/dxAgJq687GJceszrEKMDBqMTDazl9cqAQ a2JZcWXuIUYJDmYlEd4ZqVMChXhTEiurUovy44tKc1KLDzEycXBKNTDOnnXvvsxz7UvaZeI/ 7vMtrdvqtn+a2Hbd4OJlF37Oui15Kf29EPMJTpO6L9unfpu+IltZ6W/BgtgtcT5h+ifDl5Q/ XnlN/bYAQ2mg/7L5jVy7J/h2crxRXKDNtsL3wr2FdkzP/4kF6DYGSht/njP3a20ezycr/4MW wTM2WO3PjPNmi5zlYVKjxFKckWioxVxUnAgAiPd9TwkCAAA= Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2013.5.16.80021 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' HTML_NO_HTTP 0.1, MULTIPLE_RCPTS 0.1, HTML_00_10 0.05, BODY_SIZE_3000_3999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __HAS_FROM 0, __HAS_HTML 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __SANE_MSGID 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS ' |
Commit Message
Andrzej Hajda
May 16, 2013, 8:14 a.m. UTC
This patch adds managed versions of initialization function for media entity. Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- v3: - removed managed cleanup --- drivers/media/media-entity.c | 44 ++++++++++++++++++++++++++++++++++++++++++ include/media/media-entity.h | 5 +++++ 2 files changed, 49 insertions(+)
Comments
Hi Andrzej, Thank you for the patch. On Thursday 16 May 2013 10:14:32 Andrzej Hajda wrote: > This patch adds managed versions of initialization > function for media entity. > > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> > Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > v3: > - removed managed cleanup > --- > drivers/media/media-entity.c | 44 +++++++++++++++++++++++++++++++++++++++ > include/media/media-entity.h | 5 +++++ > 2 files changed, 49 insertions(+) > > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c > index e1cd132..b1e29a7 100644 > --- a/drivers/media/media-entity.c > +++ b/drivers/media/media-entity.c > @@ -82,9 +82,53 @@ void > media_entity_cleanup(struct media_entity *entity) > { > kfree(entity->links); > + entity->links = NULL; > } > EXPORT_SYMBOL_GPL(media_entity_cleanup); > > +static void devm_media_entity_release(struct device *dev, void *res) > +{ > + struct media_entity **entity = res; > + > + media_entity_cleanup(*entity); > +} > + > +/** > + * devm_media_entity_init - managed media entity initialization > + * > + * @dev: Device for which @entity belongs to. > + * @entity: Entity to be initialized. > + * @num_pads: Total number of sink and source pads. > + * @pads: Array of 'num_pads' pads. > + * @extra_links: Initial estimate of the number of extra links. > + * > + * This is a managed version of media_entity_init. Entity initialized with > + * this function will be automatically cleaned up on driver detach. > + */ > +int > +devm_media_entity_init(struct device *dev, struct media_entity *entity, > + u16 num_pads, struct media_pad *pads, u16 extra_links) What kind of users do you see for this function ? Aren't subdev drivers supposed to use the devm_* functions from patch 3/3 instead ? We should at least make it clear in the documentation that drivers must not use both devm_media_entity_init() and devm_v4l2_subdev_i2c_init(). > +{ > + struct media_entity **dr; > + int rc; > + > + dr = devres_alloc(devm_media_entity_release, sizeof(*dr), GFP_KERNEL); > + if (!dr) > + return -ENOMEM; > + > + rc = media_entity_init(entity, num_pads, pads, extra_links); > + if (rc) { > + devres_free(dr); > + return rc; > + } > + > + *dr = entity; > + devres_add(dev, dr); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(devm_media_entity_init); > + > /* > --------------------------------------------------------------------------- > -- * Graph traversal > */ > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > index 0c16f51..e25730e 100644 > --- a/include/media/media-entity.h > +++ b/include/media/media-entity.h > @@ -26,6 +26,8 @@ > #include <linux/list.h> > #include <linux/media.h> > > +struct device; > + > struct media_pipeline { > }; > > @@ -126,6 +128,9 @@ int media_entity_init(struct media_entity *entity, u16 > num_pads, struct media_pad *pads, u16 extra_links); > void media_entity_cleanup(struct media_entity *entity); > > +int devm_media_entity_init(struct device *dev, struct media_entity *entity, > + u16 num_pads, struct media_pad *pads, u16 extra_links); > + > int media_entity_create_link(struct media_entity *source, u16 source_pad, > struct media_entity *sink, u16 sink_pad, u32 flags); > int __media_entity_setup_link(struct media_link *link, u32 flags);
On 06/19/2013 12:03 AM, Laurent Pinchart wrote: > Hi Andrzej, > > Thank you for the patch. > > On Thursday 16 May 2013 10:14:32 Andrzej Hajda wrote: >> This patch adds managed versions of initialization >> function for media entity. >> >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> >> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >> --- >> v3: >> - removed managed cleanup >> --- >> drivers/media/media-entity.c | 44 +++++++++++++++++++++++++++++++++++++++ >> include/media/media-entity.h | 5 +++++ >> 2 files changed, 49 insertions(+) >> >> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c >> index e1cd132..b1e29a7 100644 >> --- a/drivers/media/media-entity.c >> +++ b/drivers/media/media-entity.c >> @@ -82,9 +82,53 @@ void >> media_entity_cleanup(struct media_entity *entity) >> { >> kfree(entity->links); >> + entity->links = NULL; >> } >> EXPORT_SYMBOL_GPL(media_entity_cleanup); >> >> +static void devm_media_entity_release(struct device *dev, void *res) >> +{ >> + struct media_entity **entity = res; >> + >> + media_entity_cleanup(*entity); >> +} >> + >> +/** >> + * devm_media_entity_init - managed media entity initialization >> + * >> + * @dev: Device for which @entity belongs to. >> + * @entity: Entity to be initialized. >> + * @num_pads: Total number of sink and source pads. >> + * @pads: Array of 'num_pads' pads. >> + * @extra_links: Initial estimate of the number of extra links. >> + * >> + * This is a managed version of media_entity_init. Entity initialized with >> + * this function will be automatically cleaned up on driver detach. >> + */ >> +int >> +devm_media_entity_init(struct device *dev, struct media_entity *entity, >> + u16 num_pads, struct media_pad *pads, u16 extra_links) > What kind of users do you see for this function ? Aren't subdev drivers > supposed to use the devm_* functions from patch 3/3 instead ? We should at > least make it clear in the documentation that drivers must not use both > devm_media_entity_init() and devm_v4l2_subdev_i2c_init(). It can be used for media entities which are not part of subdev. I will add statement about it. Besides subdev, for now only video_device uses media entity. I am not 100% sure about advantages of adding devm_media_entity_init - currently only 7 drivers in kernel uses video_device and media_entity_cleanup in those drivers is called not as straightforward as in subdevs. Replacing it with managed version would require deeper analysis :) > >> +{ >> + struct media_entity **dr; >> + int rc; >> + >> + dr = devres_alloc(devm_media_entity_release, sizeof(*dr), GFP_KERNEL); >> + if (!dr) >> + return -ENOMEM; >> + >> + rc = media_entity_init(entity, num_pads, pads, extra_links); >> + if (rc) { >> + devres_free(dr); >> + return rc; >> + } >> + >> + *dr = entity; >> + devres_add(dev, dr); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(devm_media_entity_init); >> + >> /* >> --------------------------------------------------------------------------- >> -- * Graph traversal >> */ >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h >> index 0c16f51..e25730e 100644 >> --- a/include/media/media-entity.h >> +++ b/include/media/media-entity.h >> @@ -26,6 +26,8 @@ >> #include <linux/list.h> >> #include <linux/media.h> >> >> +struct device; >> + >> struct media_pipeline { >> }; >> >> @@ -126,6 +128,9 @@ int media_entity_init(struct media_entity *entity, u16 >> num_pads, struct media_pad *pads, u16 extra_links); >> void media_entity_cleanup(struct media_entity *entity); >> >> +int devm_media_entity_init(struct device *dev, struct media_entity *entity, >> + u16 num_pads, struct media_pad *pads, u16 extra_links); >> + >> int media_entity_create_link(struct media_entity *source, u16 source_pad, >> struct media_entity *sink, u16 sink_pad, u32 flags); >> int __media_entity_setup_link(struct media_link *link, u32 flags); -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andrzej, On Wednesday 19 June 2013 12:33:11 Andrzej Hajda wrote: > On 06/19/2013 12:03 AM, Laurent Pinchart wrote: > > On Thursday 16 May 2013 10:14:32 Andrzej Hajda wrote: > >> This patch adds managed versions of initialization > >> function for media entity. > >> > >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> > >> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > >> --- > >> > >> v3: > >> - removed managed cleanup > >> > >> --- > >> > >> drivers/media/media-entity.c | 44 ++++++++++++++++++++++++++++++++++++ > >> include/media/media-entity.h | 5 +++++ > >> 2 files changed, 49 insertions(+) > >> > >> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c > >> index e1cd132..b1e29a7 100644 > >> --- a/drivers/media/media-entity.c > >> +++ b/drivers/media/media-entity.c > >> @@ -82,9 +82,53 @@ void > >> media_entity_cleanup(struct media_entity *entity) > >> { > >> kfree(entity->links); > >> + entity->links = NULL; > >> } > >> EXPORT_SYMBOL_GPL(media_entity_cleanup); > >> > >> +static void devm_media_entity_release(struct device *dev, void *res) > >> +{ > >> + struct media_entity **entity = res; > >> + > >> + media_entity_cleanup(*entity); > >> +} > >> + > >> +/** > >> + * devm_media_entity_init - managed media entity initialization > >> + * > >> + * @dev: Device for which @entity belongs to. > >> + * @entity: Entity to be initialized. > >> + * @num_pads: Total number of sink and source pads. > >> + * @pads: Array of 'num_pads' pads. > >> + * @extra_links: Initial estimate of the number of extra links. > >> + * > >> + * This is a managed version of media_entity_init. Entity initialized > >> with > >> + * this function will be automatically cleaned up on driver detach. > >> + */ > >> +int > >> +devm_media_entity_init(struct device *dev, struct media_entity *entity, > >> + u16 num_pads, struct media_pad *pads, u16 extra_links) > > > > What kind of users do you see for this function ? Aren't subdev drivers > > supposed to use the devm_* functions from patch 3/3 instead ? We should at > > least make it clear in the documentation that drivers must not use both > > devm_media_entity_init() and devm_v4l2_subdev_i2c_init(). > > It can be used for media entities which are not part of subdev. > I will add statement about it. > Besides subdev, for now only video_device uses media entity. > I am not 100% sure about advantages of adding devm_media_entity_init - > currently only 7 drivers in kernel uses video_device and > media_entity_cleanup in those drivers is called not as straightforward > as in subdevs. > Replacing it with managed version would require deeper analysis :) Thank you fhr the explanation. Maybe we should then delay introducing devm_media_entity_init until needed ?
On 06/19/2013 12:36 PM, Laurent Pinchart wrote: > Hi Andrzej, > > On Wednesday 19 June 2013 12:33:11 Andrzej Hajda wrote: >> On 06/19/2013 12:03 AM, Laurent Pinchart wrote: >>> On Thursday 16 May 2013 10:14:32 Andrzej Hajda wrote: >>>> This patch adds managed versions of initialization >>>> function for media entity. >>>> >>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com> >>>> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >>>> --- >>>> >>>> v3: >>>> - removed managed cleanup >>>> >>>> --- >>>> >>>> drivers/media/media-entity.c | 44 ++++++++++++++++++++++++++++++++++++ >>>> include/media/media-entity.h | 5 +++++ >>>> 2 files changed, 49 insertions(+) >>>> >>>> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c >>>> index e1cd132..b1e29a7 100644 >>>> --- a/drivers/media/media-entity.c >>>> +++ b/drivers/media/media-entity.c >>>> @@ -82,9 +82,53 @@ void >>>> media_entity_cleanup(struct media_entity *entity) >>>> { >>>> kfree(entity->links); >>>> + entity->links = NULL; >>>> } >>>> EXPORT_SYMBOL_GPL(media_entity_cleanup); >>>> >>>> +static void devm_media_entity_release(struct device *dev, void *res) >>>> +{ >>>> + struct media_entity **entity = res; >>>> + >>>> + media_entity_cleanup(*entity); >>>> +} >>>> + >>>> +/** >>>> + * devm_media_entity_init - managed media entity initialization >>>> + * >>>> + * @dev: Device for which @entity belongs to. >>>> + * @entity: Entity to be initialized. >>>> + * @num_pads: Total number of sink and source pads. >>>> + * @pads: Array of 'num_pads' pads. >>>> + * @extra_links: Initial estimate of the number of extra links. >>>> + * >>>> + * This is a managed version of media_entity_init. Entity initialized >>>> with >>>> + * this function will be automatically cleaned up on driver detach. >>>> + */ >>>> +int >>>> +devm_media_entity_init(struct device *dev, struct media_entity *entity, >>>> + u16 num_pads, struct media_pad *pads, u16 extra_links) >>> What kind of users do you see for this function ? Aren't subdev drivers >>> supposed to use the devm_* functions from patch 3/3 instead ? We should at >>> least make it clear in the documentation that drivers must not use both >>> devm_media_entity_init() and devm_v4l2_subdev_i2c_init(). >> It can be used for media entities which are not part of subdev. >> I will add statement about it. >> Besides subdev, for now only video_device uses media entity. >> I am not 100% sure about advantages of adding devm_media_entity_init - >> currently only 7 drivers in kernel uses video_device and >> media_entity_cleanup in those drivers is called not as straightforward >> as in subdevs. >> Replacing it with managed version would require deeper analysis :) > Thank you fhr the explanation. Maybe we should then delay introducing > devm_media_entity_init until needed ? > Yes, it can be delayed. Regards Andrzej -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c index e1cd132..b1e29a7 100644 --- a/drivers/media/media-entity.c +++ b/drivers/media/media-entity.c @@ -82,9 +82,53 @@ void media_entity_cleanup(struct media_entity *entity) { kfree(entity->links); + entity->links = NULL; } EXPORT_SYMBOL_GPL(media_entity_cleanup); +static void devm_media_entity_release(struct device *dev, void *res) +{ + struct media_entity **entity = res; + + media_entity_cleanup(*entity); +} + +/** + * devm_media_entity_init - managed media entity initialization + * + * @dev: Device for which @entity belongs to. + * @entity: Entity to be initialized. + * @num_pads: Total number of sink and source pads. + * @pads: Array of 'num_pads' pads. + * @extra_links: Initial estimate of the number of extra links. + * + * This is a managed version of media_entity_init. Entity initialized with + * this function will be automatically cleaned up on driver detach. + */ +int +devm_media_entity_init(struct device *dev, struct media_entity *entity, + u16 num_pads, struct media_pad *pads, u16 extra_links) +{ + struct media_entity **dr; + int rc; + + dr = devres_alloc(devm_media_entity_release, sizeof(*dr), GFP_KERNEL); + if (!dr) + return -ENOMEM; + + rc = media_entity_init(entity, num_pads, pads, extra_links); + if (rc) { + devres_free(dr); + return rc; + } + + *dr = entity; + devres_add(dev, dr); + + return 0; +} +EXPORT_SYMBOL_GPL(devm_media_entity_init); + /* ----------------------------------------------------------------------------- * Graph traversal */ diff --git a/include/media/media-entity.h b/include/media/media-entity.h index 0c16f51..e25730e 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -26,6 +26,8 @@ #include <linux/list.h> #include <linux/media.h> +struct device; + struct media_pipeline { }; @@ -126,6 +128,9 @@ int media_entity_init(struct media_entity *entity, u16 num_pads, struct media_pad *pads, u16 extra_links); void media_entity_cleanup(struct media_entity *entity); +int devm_media_entity_init(struct device *dev, struct media_entity *entity, + u16 num_pads, struct media_pad *pads, u16 extra_links); + int media_entity_create_link(struct media_entity *source, u16 source_pad, struct media_entity *sink, u16 sink_pad, u32 flags); int __media_entity_setup_link(struct media_link *link, u32 flags);