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@xxxxxxxxxxx>
>> Reviewed-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
>> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
