Em Thu, 13 Dec 2018 14:41:12 +0100
hverkuil-cisco@xxxxxxxxx escreveu:
> From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
>
> Add functions to add properties to entities, pads and other
> properties. This can be extended to include interfaces and links
> in the future when needed.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
> drivers/media/media-entity.c | 64 +++++++++
> include/media/media-entity.h | 264 +++++++++++++++++++++++++++++++++++
> 2 files changed, 328 insertions(+)
>
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 62c4d5b4d33f..cdb35bc8e9a0 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -251,6 +251,70 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> }
> EXPORT_SYMBOL_GPL(media_entity_pads_init);
>
> +static struct media_prop *media_create_prop(struct media_gobj *owner, u32 type,
> + const char *name, const void *ptr,
> + u32 payload_size)
> +{
> + struct media_prop *prop = kzalloc(sizeof(*prop) + payload_size,
> + GFP_KERNEL);
> +
> + if (!prop)
> + return ERR_PTR(-ENOMEM);
> + prop->type = type;
> + strscpy(prop->name, name, sizeof(prop->name));
> + if (owner->mdev)
> + media_gobj_create(owner->mdev, MEDIA_GRAPH_PROP,
> + &prop->graph_obj);
> + prop->owner = owner;
> + prop->payload_size = payload_size;
> + if (payload_size && ptr)
> + memcpy(prop->payload, ptr, payload_size);
> + INIT_LIST_HEAD(&prop->props);
> + return prop;
> +}
> +
> +struct media_prop *media_entity_add_prop(struct media_entity *ent, u32 type,
> + const char *name, const void *ptr,
> + u32 payload_size)
> +{
> + struct media_prop *prop;
> +
> + if (!ent->inited)
> + media_entity_init(ent);
> + prop = media_create_prop(&ent->graph_obj, type,
> + name, ptr, payload_size);
> + if (!IS_ERR(prop))
> + list_add_tail(&prop->list, &ent->props);
> + return prop;
> +}
> +EXPORT_SYMBOL_GPL(media_entity_add_prop);
> +
> +struct media_prop *media_pad_add_prop(struct media_pad *pad, u32 type,
> + const char *name, const void *ptr,
> + u32 payload_size)
> +{
> + struct media_prop *prop = media_create_prop(&pad->graph_obj, type,
> + name, ptr, payload_size);
> +
> + if (!IS_ERR(prop))
> + list_add_tail(&prop->list, &pad->props);
> + return prop;
> +}
> +EXPORT_SYMBOL_GPL(media_pad_add_prop);
> +
> +struct media_prop *media_prop_add_prop(struct media_prop *prop, u32 type,
> + const char *name, const void *ptr,
> + u32 payload_size)
> +{
> + struct media_prop *subprop = media_create_prop(&prop->graph_obj, type,
> + name, ptr, payload_size);
> +
> + if (!IS_ERR(subprop))
> + list_add_tail(&subprop->list, &prop->props);
> + return subprop;
> +}
> +EXPORT_SYMBOL_GPL(media_prop_add_prop);
> +
> /* -----------------------------------------------------------------------------
> * Graph traversal
> */
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 5d05ebf712d0..695acfd3fe9c 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -833,6 +833,270 @@ int media_create_pad_links(const struct media_device *mdev,
>
> void __media_entity_remove_links(struct media_entity *entity);
>
> +/**
> + * media_entity_add_prop() - Add property to entity
> + *
> + * @entity: entity where to add the property
> + * @type: property type
> + * @name: property name
> + * @ptr: property pointer to payload
> + * @payload_size: property payload size
> + *
> + * Returns the new property on success, or an error pointer on failure.
> + */
> +struct media_prop *media_entity_add_prop(struct media_entity *ent, u32 type,
> + const char *name, const void *ptr,
> + u32 payload_size);
> +
> +/**
> + * media_pad_add_prop() - Add property to pad
> + *
> + * @pad: pad where to add the property
> + * @type: property type
> + * @name: property name
> + * @ptr: property pointer to payload
> + * @payload_size: property payload size
> + *
> + * Returns the new property on success, or an error pointer on failure.
> + */
> +struct media_prop *media_pad_add_prop(struct media_pad *pad, u32 type,
> + const char *name, const void *ptr,
> + u32 payload_size);
> +
> +/**
> + * media_prop() - Add sub-property to property
> + *
> + * @prop: property where to add the sub-property
> + * @type: sub-property type
> + * @name: sub-property name
> + * @ptr: sub-property pointer to payload
> + * @payload_size: sub-property payload size
> + *
> + * Returns the new property on success, or an error pointer on failure.
> + */
> +struct media_prop *media_prop_add_prop(struct media_prop *prop, u32 type,
> + const char *name, const void *ptr,
> + u32 payload_size);
> +
> +/**
> + * media_entity_add_prop_group() - Add group property to entity
> + *
> + * @entity: entity where to add the property
> + * @name: property name
> + *
> + * Returns the new property on success, or an error pointer on failure.
> + */
> +static inline struct media_prop *
> +media_entity_add_prop_group(struct media_entity *entity, const char *name)
> +{
> + return media_entity_add_prop(entity, MEDIA_PROP_TYPE_GROUP,
> + name, NULL, 0);
> +}
Hmm... group is just a string? On my past comments for patches 1/4 and 2/4
I assumed it would be an u32 array :-)
That's btw why I asked you to add some documentation. That would avoid
reviewers of guessing some things ;-)
Why are you using zero for the payload? At patch 1/4, you said that
only u64 and s64 would have payload_size == 0:
+ * @payload_size: Property payload size, 0 for U64/S64
One would infer that, for all other types, payload_size would be
a non-zero value.
> +
> +/**
> + * media_entity_add_prop_u64() - Add u64 property to entity
> + *
> + * @entity: entity where to add the property
> + * @name: property name
> + * @val: property value
> + *
> + * Returns 0 on success, or an error on failure.
> + */
> +static inline int media_entity_add_prop_u64(struct media_entity *entity,
> + const char *name, u64 val)
> +{
> + struct media_prop *prop =
> + media_entity_add_prop(entity, MEDIA_PROP_TYPE_U64,
> + name, &val, sizeof(val));
> +
> + return PTR_ERR_OR_ZERO(prop);
> +}
> +
> +/**
> + * media_entity_add_prop_s64() - Add s64 property to entity
> + *
> + * @entity: entity where to add the property
> + * @name: property name
> + * @val: property value
> + *
> + * Returns 0 on success, or an error on failure.
> + */
> +static inline int media_entity_add_prop_s64(struct media_entity *entity,
> + const char *name, s64 val)
> +{
> + struct media_prop *prop =
> + media_entity_add_prop(entity, MEDIA_PROP_TYPE_S64,
> + name, &val, sizeof(val));
> +
> + return PTR_ERR_OR_ZERO(prop);
> +}
> +
> +/**
> + * media_entity_add_prop_string() - Add string property to entity
> + *
> + * @entity: entity where to add the property
> + * @name: property name
> + * @string: property string value
> + *
> + * Returns 0 on success, or an error on failure.
> + */
> +static inline int media_entity_add_prop_string(struct media_entity *entity,
> + const char *name,
> + const char *string)
> +{
> + struct media_prop *prop =
> + media_entity_add_prop(entity, MEDIA_PROP_TYPE_STRING,
> + name, string, strlen(string) + 1);
Ok, so, you're assuming that, for strings, payload_type will include
the ending '\0' character, and that strings are NUL-terminated.
Please document it, as we usually have NUL-terminated strings OR
LEN + string. Having both is non-trivial.
> +
> + return PTR_ERR_OR_ZERO(prop);
> +}
> +
> +/**
> + * media_pad_add_prop_group() - Add group property to pad
> + *
> + * @pad: pad where to add the property
> + * @name: property name
> + *
> + * Returns the new property on success, or an error pointer on failure.
> + */
> +static inline struct media_prop *media_pad_add_prop_group(struct media_pad *pad,
> + const char *name)
> +{
> + return media_pad_add_prop(pad, MEDIA_PROP_TYPE_GROUP,
> + name, NULL, 0);
> +}
> +
> +/**
> + * media_pad_add_prop_u64() - Add u64 property to pad
> + *
> + * @pad: pad where to add the property
> + * @name: property name
> + * @val: property value
> + *
> + * Returns 0 on success, or an error on failure.
> + */
> +static inline int media_pad_add_prop_u64(struct media_pad *pad,
> + const char *name, u64 val)
> +{
> + struct media_prop *prop =
> + media_pad_add_prop(pad, MEDIA_PROP_TYPE_U64,
> + name, &val, sizeof(val));
> +
> + return PTR_ERR_OR_ZERO(prop);
> +}
> +
> +/**
> + * media_pad_add_prop_s64() - Add s64 property to pad
> + *
> + * @pad: pad where to add the property
> + * @name: property name
> + * @val: property value
> + *
> + * Returns 0 on success, or an error on failure.
> + */
> +static inline int media_pad_add_prop_s64(struct media_pad *pad,
> + const char *name, s64 val)
> +{
> + struct media_prop *prop =
> + media_pad_add_prop(pad, MEDIA_PROP_TYPE_S64,
> + name, &val, sizeof(val));
> +
> + return PTR_ERR_OR_ZERO(prop);
> +}
> +
> +/**
> + * media_pad_add_prop_string() - Add string property to pad
> + *
> + * @pad: pad where to add the property
> + * @name: property name
> + * @string: property string value
> + *
> + * Returns 0 on success, or an error on failure.
> + */
> +static inline int media_pad_add_prop_string(struct media_pad *pad,
> + const char *name,
> + const char *string)
> +{
> + struct media_prop *prop =
> + media_pad_add_prop(pad, MEDIA_PROP_TYPE_STRING,
> + name, string, strlen(string) + 1);
> +
> + return PTR_ERR_OR_ZERO(prop);
> +}
> +
> +/**
> + * media_prop_add_prop_group() - Add group sub-property to property
> + *
> + * @prop: property where to add the sub-property
> + * @name: sub-property name
> + *
> + * Returns the new property on success, or an error pointer on failure.
> + */
> +static inline struct media_prop *
> +media_prop_add_prop_group(struct media_prop *prop, const char *name)
> +{
> + return media_prop_add_prop(prop, MEDIA_PROP_TYPE_GROUP,
> + name, NULL, 0);
> +}
> +
> +/**
> + * media_prop_add_prop_u64() - Add u64 property to property
> + *
> + * @prop: property where to add the sub-property
> + * @name: sub-property name
> + * @val: sub-property value
> + *
> + * Returns 0 on success, or an error on failure.
> + */
> +static inline int media_prop_add_prop_u64(struct media_prop *prop,
> + const char *name, u64 val)
> +{
> + struct media_prop *subprop =
> + media_prop_add_prop(prop, MEDIA_PROP_TYPE_U64,
> + name, &val, sizeof(val));
> +
> + return PTR_ERR_OR_ZERO(subprop);
> +}
> +
> +/**
> + * media_prop_add_prop_s64() - Add s64 property to property
> + *
> + * @prop: property where to add the sub-property
> + * @name: sub-property name
> + * @val: sub-property value
> + *
> + * Returns 0 on success, or an error on failure.
> + */
> +static inline int media_prop_add_prop_s64(struct media_prop *prop,
> + const char *name, s64 val)
> +{
> + struct media_prop *subprop =
> + media_prop_add_prop(prop, MEDIA_PROP_TYPE_S64,
> + name, &val, sizeof(val));
> +
> + return PTR_ERR_OR_ZERO(subprop);
> +}
> +
> +/**
> + * media_prop_add_prop_string() - Add string property to property
> + *
> + * @prop: property where to add the sub-property
> + * @name: sub-property name
> + * @string: sub-property string value
> + *
> + * Returns 0 on success, or an error on failure.
> + */
> +static inline int media_prop_add_prop_string(struct media_prop *prop,
> + const char *name,
> + const char *string)
> +{
> + struct media_prop *subprop =
> + media_prop_add_prop(prop, MEDIA_PROP_TYPE_STRING,
> + name, string, strlen(string) + 1);
> +
> + return PTR_ERR_OR_ZERO(subprop);
> +}
> +
> /**
> * media_entity_remove_links() - remove all links associated with an entity
> *
Thanks,
Mauro
