Em Thu, 13 Dec 2018 18:35:15 +0100
Hans Verkuil <hverkuil-cisco@xxxxxxxxx> escreveu:
> On 12/13/18 6:14 PM, Mauro Carvalho Chehab wrote:
> > Em Thu, 13 Dec 2018 14:41:11 +0100
> > hverkuil-cisco@xxxxxxxxx escreveu:
> >
> >> From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> >>
> >> Add support for properties. In this initial implementation properties
> >> can be added to entities and pads. In addition, properties can be
> >> nested.
> >>
> >> Most of the changes are straightforward, but I had to make some changes
> >> to the way entities are initialized, since the struct has to be
> >> initialized either when media_entity_pads_init() is called or when
> >> media_device_register_entity() is called, whichever comes first.
> >>
> >> The properties list in the entity has to be initialized in both cases,
> >> otherwise you can't add properties to it.
> >>
> >> The entity struct is now initialized in media_entity_init and called
> >> from both functions if ent->inited is 0.
> >>
> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> >> ---
> >> drivers/media/media-device.c | 129 ++++++++++++++++++++++++++++++++---
> >> drivers/media/media-entity.c | 26 +++++--
> >> include/media/media-device.h | 6 ++
> >> include/media/media-entity.h | 58 ++++++++++++++++
> >> 4 files changed, 205 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> >> index bed24372e61f..a932e6848d47 100644
> >> --- a/drivers/media/media-device.c
> >> +++ b/drivers/media/media-device.c
> >> @@ -242,10 +242,14 @@ static long media_device_get_topology(struct media_device *mdev, void *arg)
> >> struct media_interface *intf;
> >> struct media_pad *pad;
> >> struct media_link *link;
> >> + struct media_prop *prop;
> >> struct media_v2_entity kentity, __user *uentity;
> >> struct media_v2_interface kintf, __user *uintf;
> >> struct media_v2_pad kpad, __user *upad;
> >> struct media_v2_link klink, __user *ulink;
> >> + struct media_v2_prop kprop, __user *uprop;
> >> + unsigned int props_payload_size = 0;
> >> + unsigned int prop_payload_offset;
> >> unsigned int i;
> >> int ret = 0;
> >>
> >> @@ -375,6 +379,48 @@ static long media_device_get_topology(struct media_device *mdev, void *arg)
> >> topo->num_links = i;
> >> topo->reserved4 = 0;
> >>
> >> + /* Get properties and number of properties */
> >> + i = 0;
> >> + uprop = media_get_uptr(topo->ptr_props);
> >> + media_device_for_each_prop(prop, mdev) {
> >> + i++;
> >> + props_payload_size += ALIGN(prop->payload_size, sizeof(u64));
> >
> > I wouldn't be aligning it to u64. Instead, I would use something like:
> >
> > ALIGN(prop->payload_size, sizeof(void *))
> >
> > This way, it would waste less space on 32-bits CPU.
>
> OK.
>
> >
> >> + }
> >> + if (i > topo->num_props ||
> >> + props_payload_size > topo->props_payload_size)
> >> + ret = ret ? : -ENOSPC;
> >> + topo->props_payload_size = props_payload_size;
> >> + topo->num_props = i;
> >> + topo->reserved4 = 0;
> >> +
> >> + if (ret || !uprop)
> >> + return ret;
> >> +
> >> + prop_payload_offset = topo->num_props * sizeof(*uprop);
> >> + media_device_for_each_prop(prop, mdev) {
> >> + memset(&kprop, 0, sizeof(kprop));
> >> +
> >> + /* Copy prop fields to userspace struct */
> >> + kprop.id = prop->graph_obj.id;
> >> + kprop.type = prop->type;
> >> + kprop.owner_id = prop->owner->id;
> >> + kprop.owner_type = media_type(prop->owner);
> >> + kprop.payload_size = prop->payload_size;
> >> + if (prop->payload_size) {
> >> + kprop.payload_offset = prop_payload_offset;
> >> + if (copy_to_user((u8 __user *)uprop + prop_payload_offset,
> >> + prop->payload, prop->payload_size))
> >> + return -EFAULT;
> >> + prop_payload_offset += ALIGN(prop->payload_size, sizeof(u64));
> >> + }
> >> + memcpy(kprop.name, prop->name, sizeof(kprop.name));
> >> +
> >> + if (copy_to_user(uprop, &kprop, sizeof(kprop)))
> >> + return -EFAULT;
> >> + uprop++;
> >> + prop_payload_offset -= sizeof(*uprop);
> >> + }
> >> +
> >> return ret;
> >> }
> >>
> >> @@ -408,9 +454,10 @@ static long copy_arg_to_user(void __user *uarg, void *karg, unsigned int cmd)
> >> /* Do acquire the graph mutex */
> >> #define MEDIA_IOC_FL_GRAPH_MUTEX BIT(0)
> >>
> >> -#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
> >> +#define MEDIA_IOC_ARG(__cmd, alts, func, fl, from_user, to_user) \
> >> [_IOC_NR(MEDIA_IOC_##__cmd)] = { \
> >> .cmd = MEDIA_IOC_##__cmd, \
> >> + .alternatives = (alts), \
> >> .fn = (long (*)(struct media_device *, void *))func, \
> >> .flags = fl, \
> >> .arg_from_user = from_user, \
> >> @@ -418,23 +465,39 @@ static long copy_arg_to_user(void __user *uarg, void *karg, unsigned int cmd)
> >> }
> >>
> >> #define MEDIA_IOC(__cmd, func, fl) \
> >> - MEDIA_IOC_ARG(__cmd, func, fl, copy_arg_from_user, copy_arg_to_user)
> >> + MEDIA_IOC_ARG(__cmd, NULL, func, fl, copy_arg_from_user, copy_arg_to_user)
> >> +#define MEDIA_IOC_ALTS(__cmd, alts, func, fl) \
> >> + MEDIA_IOC_ARG(__cmd, alts, func, fl, copy_arg_from_user, copy_arg_to_user)
> >>
> >> /* the table is indexed by _IOC_NR(cmd) */
> >> struct media_ioctl_info {
> >> unsigned int cmd;
> >> + const unsigned int *alternatives;
> >> unsigned short flags;
> >> long (*fn)(struct media_device *dev, void *arg);
> >> long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
> >> long (*arg_to_user)(void __user *uarg, void *karg, unsigned int cmd);
> >> };
> >>
> >> +#define _IOWR_COMPAT(type, nr, size) \
> >> + _IOC(_IOC_READ | _IOC_WRITE, (type), (nr), (size))
> >> +
> >> +/* Old MEDIA_IOC_G_TOPOLOGY ioctl without props support */
> >> +#define MEDIA_IOC_G_TOPOLOGY_V1 _IOWR_COMPAT('|', 0x04, offsetof(struct media_v2_topology, num_props))
> >> +
> >> +static const unsigned int topo_alts[] = {
> >> + /* Old MEDIA_IOC_G_TOPOLOGY without props support */
> >> + MEDIA_IOC_G_TOPOLOGY_V1,
> >> + 0
> >> +};
> >> +
> >> static const struct media_ioctl_info ioctl_info[] = {
> >> MEDIA_IOC(DEVICE_INFO, media_device_get_info, MEDIA_IOC_FL_GRAPH_MUTEX),
> >> MEDIA_IOC(ENUM_ENTITIES, media_device_enum_entities, MEDIA_IOC_FL_GRAPH_MUTEX),
> >> MEDIA_IOC(ENUM_LINKS, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX),
> >> MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
> >> MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
> >> + MEDIA_IOC_ALTS(G_TOPOLOGY, topo_alts, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
> >> MEDIA_IOC(REQUEST_ALLOC, media_device_request_alloc, 0),
> >> };
> >>
> >> @@ -448,17 +511,29 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
> >> char __karg[256], *karg = __karg;
> >> long ret;
> >>
> >> - if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info)
> >> - || ioctl_info[_IOC_NR(cmd)].cmd != cmd)
> >> + if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info))
> >> return -ENOIOCTLCMD;
> >>
> >> info = &ioctl_info[_IOC_NR(cmd)];
> >>
> >> + if (info->cmd != cmd) {
> >> + const unsigned int *p;
> >> +
> >> + for (p = info->alternatives; p && *p; p++)
> >> + if (*p == cmd)
> >> + break;
> >> + if (!p || !*p)
> >> + return -ENOIOCTLCMD;
> >> + }
> >> +
> >> if (_IOC_SIZE(info->cmd) > sizeof(__karg)) {
> >> karg = kmalloc(_IOC_SIZE(info->cmd), GFP_KERNEL);
> >> if (!karg)
> >> return -ENOMEM;
> >> }
> >> + if (_IOC_SIZE(info->cmd) > _IOC_SIZE(cmd))
> >> + memset(karg + _IOC_SIZE(cmd), 0,
> >> + _IOC_SIZE(info->cmd) - _IOC_SIZE(cmd));
> >>
> >> if (info->arg_from_user) {
> >> ret = info->arg_from_user(karg, arg, cmd);
> >> @@ -571,6 +646,16 @@ static void media_device_release(struct media_devnode *devnode)
> >> dev_dbg(devnode->parent, "Media device released\n");
> >> }
> >>
> >> +static void init_prop_list(struct media_device *mdev, struct list_head *list)
> >> +{
> >> + struct media_prop *prop;
> >> +
> >> + list_for_each_entry(prop, list, list) {
> >> + media_gobj_create(mdev, MEDIA_GRAPH_PROP, &prop->graph_obj);
> >> + init_prop_list(mdev, &prop->props);
> >> + }
> >> +}
> >> +
> >> /**
> >> * media_device_register_entity - Register an entity with a media device
> >> * @mdev: The media device
> >> @@ -592,9 +677,8 @@ int __must_check media_device_register_entity(struct media_device *mdev,
> >> /* Warn if we apparently re-register an entity */
> >> WARN_ON(entity->graph_obj.mdev != NULL);
> >
> > It seems that you missed my comments on this hunk (or maybe I missed your
> > answer to my review of patch 2/3 of RFCv4[1]).
>
> Ah yes, I forgot that. I was a bit in a hurry to prepare a v5 since this is the last one
> before my vacation.
>
> >
> > [1] https://lore.kernel.org/linux-media/20181212070225.2863a72e@xxxxxxxx/
> >
> > I'll repeat it here:
> >
> > For this:
> >
> > > /* Warn if we apparently re-register an entity */
> > > WARN_ON(entity->graph_obj.mdev != NULL);
> >
> > Side note: it would probably make sense to change the above to:
> >
> > if (WARN_ON(entity->graph_obj.mdev != NULL))
> > return -EINVAL;
> >
> > That should be on a separate patch, as it is unrelated to the
> > properties API addition.
> >
> >> entity->graph_obj.mdev = mdev;
> >> - INIT_LIST_HEAD(&entity->links);
> >> - entity->num_links = 0;
> >> - entity->num_backlinks = 0;
> >> + if (!entity->inited)
> >> + media_entity_init(entity);
> >
> > Nitpick: I would add a comment for this explaining why the check is
> > required here.
>
> Good point.
>
> >
> >>
> >> ret = ida_alloc_min(&mdev->entity_internal_idx, 1, GFP_KERNEL);
> >> if (ret < 0)
> >> @@ -608,10 +692,17 @@ int __must_check media_device_register_entity(struct media_device *mdev,
> >> /* Initialize media_gobj embedded at the entity */
> >> media_gobj_create(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
> >>
> >> + /* Initialize objects at the props */
> >> + init_prop_list(mdev, &entity->props);
> >> +
> >> /* Initialize objects at the pads */
> >> - for (i = 0; i < entity->num_pads; i++)
> >> + for (i = 0; i < entity->num_pads; i++) {
> >> media_gobj_create(mdev, MEDIA_GRAPH_PAD,
> >> - &entity->pads[i].graph_obj);
> >> + &entity->pads[i].graph_obj);
> >> +
> >> + /* Initialize objects at the pad props */
> >> + init_prop_list(mdev, &entity->pads[i].props);
> >> + }
> >>
> >> /* invoke entity_notify callbacks */
> >> list_for_each_entry_safe(notify, next, &mdev->entity_notify, list)
> >> @@ -640,6 +731,18 @@ int __must_check media_device_register_entity(struct media_device *mdev,
> >> }
> >> EXPORT_SYMBOL_GPL(media_device_register_entity);
> >>
> >> +static void media_device_free_props(struct list_head *list)
> >> +{
> >> + while (!list_empty(list)) {
> >> + struct media_prop *prop;
> >> +
> >> + prop = list_first_entry(list, struct media_prop, list);
> >> + list_del(&prop->list);
> >> + media_gobj_destroy(&prop->graph_obj);
> >> + kfree(prop);
> >> + }
> >> +}
> >> +
> >> static void __media_device_unregister_entity(struct media_entity *entity)
> >> {
> >> struct media_device *mdev = entity->graph_obj.mdev;
> >> @@ -661,8 +764,13 @@ static void __media_device_unregister_entity(struct media_entity *entity)
> >> __media_entity_remove_links(entity);
> >>
> >> /* Remove all pads that belong to this entity */
> >> - for (i = 0; i < entity->num_pads; i++)
> >> + for (i = 0; i < entity->num_pads; i++) {
> >> + media_device_free_props(&entity->pads[i].props);
> >> media_gobj_destroy(&entity->pads[i].graph_obj);
> >> + }
> >> +
> >> + /* Remove all props that belong to this entity */
> >> + media_device_free_props(&entity->props);
> >>
> >> /* Remove the entity */
> >> media_gobj_destroy(&entity->graph_obj);
> >> @@ -701,6 +809,7 @@ void media_device_init(struct media_device *mdev)
> >> INIT_LIST_HEAD(&mdev->interfaces);
> >> INIT_LIST_HEAD(&mdev->pads);
> >> INIT_LIST_HEAD(&mdev->links);
> >> + INIT_LIST_HEAD(&mdev->props);
> >> INIT_LIST_HEAD(&mdev->entity_notify);
> >>
> >> mutex_init(&mdev->req_queue_mutex);
> >> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> >> index 0b1cb3559140..62c4d5b4d33f 100644
> >> --- a/drivers/media/media-entity.c
> >> +++ b/drivers/media/media-entity.c
> >> @@ -34,6 +34,8 @@ static inline const char *gobj_type(enum media_gobj_type type)
> >> return "link";
> >> case MEDIA_GRAPH_INTF_DEVNODE:
> >> return "intf-devnode";
> >> + case MEDIA_GRAPH_PROP:
> >> + return "prop";
> >> default:
> >> return "unknown";
> >> }
> >
> >
> >
> >> @@ -106,7 +108,7 @@ static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj)
> >> switch (media_type(gobj)) {
> >> case MEDIA_GRAPH_ENTITY:
> >> dev_dbg(gobj->mdev->dev,
> >> - "%s id %u: entity '%s'\n",
> >> + "%s id 0x%08x: entity '%s'\n",
> >> event_name, media_id(gobj),
> >> gobj_to_entity(gobj)->name);
> >> break;
> >> @@ -115,7 +117,7 @@ static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj)
> >> struct media_link *link = gobj_to_link(gobj);
> >>
> >> dev_dbg(gobj->mdev->dev,
> >> - "%s id %u: %s link id %u ==> id %u\n",
> >> + "%s id 0x%08x: %s link id 0x%08x ==> id 0x%08x\n",
> >> event_name, media_id(gobj),
> >> media_type(link->gobj0) == MEDIA_GRAPH_PAD ?
> >> "data" : "interface",
> >> @@ -128,7 +130,7 @@ static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj)
> >> struct media_pad *pad = gobj_to_pad(gobj);
> >>
> >> dev_dbg(gobj->mdev->dev,
> >> - "%s id %u: %s%spad '%s':%d\n",
> >> + "%s id 0x%08x: %s%spad '%s':%d\n",
> >> event_name, media_id(gobj),
> >> pad->flags & MEDIA_PAD_FL_SINK ? "sink " : "",
> >> pad->flags & MEDIA_PAD_FL_SOURCE ? "source " : "",
> >> @@ -141,12 +143,22 @@ static void dev_dbg_obj(const char *event_name, struct media_gobj *gobj)
> >> struct media_intf_devnode *devnode = intf_to_devnode(intf);
> >>
> >> dev_dbg(gobj->mdev->dev,
> >> - "%s id %u: intf_devnode %s - major: %d, minor: %d\n",
> >> + "%s id 0x%08x: intf_devnode %s - major: %d, minor: %d\n",
> >> event_name, media_id(gobj),
> >> intf_type(intf),
> >> devnode->major, devnode->minor);
> >> break;
> >> }
> >
> > I also suggested to move the above hunks to a separate patch, as this change
> > is not really related to the addition of the properties.
>
> Missed that too, sorry.
>
> >
> >> + case MEDIA_GRAPH_PROP:
> >> + {
> >> + struct media_prop *prop = gobj_to_prop(gobj);
> >> +
> >> + dev_dbg(gobj->mdev->dev,
> >> + "%s id 0x%08x: prop '%s':0x%08x\n",
> >> + event_name, media_id(gobj),
> >> + prop->name, media_id(prop->owner));
> >> + break;
> >> + }
> >> }
> >> #endif
> >> }
> >> @@ -175,6 +187,9 @@ void media_gobj_create(struct media_device *mdev,
> >> case MEDIA_GRAPH_INTF_DEVNODE:
> >> list_add_tail(&gobj->list, &mdev->interfaces);
> >> break;
> >> + case MEDIA_GRAPH_PROP:
> >> + list_add_tail(&gobj->list, &mdev->props);
> >> + break;
> >> }
> >>
> >> mdev->topology_version++;
> >> @@ -212,6 +227,8 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> >> if (num_pads >= MEDIA_ENTITY_MAX_PADS)
> >> return -E2BIG;
> >>
> >> + if (!entity->inited)
> >> + media_entity_init(entity);
> >> entity->num_pads = num_pads;
> >> entity->pads = pads;
> >>
> >> @@ -221,6 +238,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> >> for (i = 0; i < num_pads; i++) {
> >> pads[i].entity = entity;
> >> pads[i].index = i;
> >> + INIT_LIST_HEAD(&pads[i].props);
> >> if (mdev)
> >> media_gobj_create(mdev, MEDIA_GRAPH_PAD,
> >> &entity->pads[i].graph_obj);
> >> diff --git a/include/media/media-device.h b/include/media/media-device.h
> >> index c8ddbfe8b74c..d422a1e1c367 100644
> >> --- a/include/media/media-device.h
> >> +++ b/include/media/media-device.h
> >> @@ -101,6 +101,7 @@ struct media_device_ops {
> >> * @interfaces: List of registered interfaces
> >> * @pads: List of registered pads
> >> * @links: List of registered links
> >> + * @props: List of registered properties
> >> * @entity_notify: List of registered entity_notify callbacks
> >> * @graph_mutex: Protects access to struct media_device data
> >> * @pm_count_walk: Graph walk for power state walk. Access serialised using
> >> @@ -170,6 +171,7 @@ struct media_device {
> >> struct list_head interfaces;
> >> struct list_head pads;
> >> struct list_head links;
> >> + struct list_head props;
> >>
> >> /* notify callback list invoked when a new entity is registered */
> >> struct list_head entity_notify;
> >> @@ -411,6 +413,10 @@ void media_device_unregister_entity_notify(struct media_device *mdev,
> >> #define media_device_for_each_link(link, mdev) \
> >> list_for_each_entry(link, &(mdev)->links, graph_obj.list)
> >>
> >> +/* Iterate over all props. */
> >> +#define media_device_for_each_prop(prop, mdev) \
> >> + list_for_each_entry(prop, &(mdev)->props, graph_obj.list)
> >> +
> >> /**
> >> * media_device_pci_init() - create and initialize a
> >> * struct &media_device from a PCI device.
> >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> >> index e5f6960d92f6..5d05ebf712d0 100644
> >> --- a/include/media/media-entity.h
> >> +++ b/include/media/media-entity.h
> >> @@ -21,6 +21,7 @@
> >>
> >> #include <linux/bitmap.h>
> >> #include <linux/bug.h>
> >> +#include <linux/err.h>
> >> #include <linux/fwnode.h>
> >> #include <linux/kernel.h>
> >> #include <linux/list.h>
> >> @@ -36,12 +37,14 @@
> >> * @MEDIA_GRAPH_LINK: Identify a media link
> >> * @MEDIA_GRAPH_INTF_DEVNODE: Identify a media Kernel API interface via
> >> * a device node
> >> + * @MEDIA_GRAPH_PROP: Identify a media property
> >> */
> >> enum media_gobj_type {
> >> MEDIA_GRAPH_ENTITY,
> >> MEDIA_GRAPH_PAD,
> >> MEDIA_GRAPH_LINK,
> >> MEDIA_GRAPH_INTF_DEVNODE,
> >> + MEDIA_GRAPH_PROP,
> >> };
> >>
> >> #define MEDIA_BITS_PER_TYPE 8
> >> @@ -193,6 +196,7 @@ enum media_pad_signal_type {
> >> * @flags: Pad flags, as defined in
> >> * :ref:`include/uapi/linux/media.h <media_header>`
> >> * (seek for ``MEDIA_PAD_FL_*``)
> >> + * @props: The list pad properties
> >> */
> >> struct media_pad {
> >> struct media_gobj graph_obj; /* must be first field in struct */
> >> @@ -200,6 +204,33 @@ struct media_pad {
> >> u16 index;
> >> enum media_pad_signal_type sig_type;
> >> unsigned long flags;
> >> + struct list_head props;
> >> +};
> >> +
> >> +/**
> >> + * struct media_prop - A media property graph object.
> >> + *
> >> + * @graph_obj: Embedded structure containing the media object common data
> >> + * @list: Linked list associated with the object that owns the link.
> >> + * @owner: Graph object this property belongs to
> >> + * @index: Property index for the owner property array, numbered
> >> + * from 0 to n
> >> + * @type: Property type
> >> + * @payload_size: Property payload size (i.e. additional bytes beyond this
> >> + * struct)
> >> + * @props: The list of sub-properties
> >> + * @name: Property name
> >> + * @payload: Property payload starts here
> >> + */
> >> +struct media_prop {
> >> + struct media_gobj graph_obj; /* must be first field in struct */
> >> + struct list_head list;
> >> + struct media_gobj *owner;
> >
> > OK.
> >
> > Despite my comment at uAPI about the N:1 case of removing the owner
> > there, here, I would keep it.
> >
> > For the first version of properties API, we should stick with 1:1 map.
> >
> > We can later expand for N:1 if needed - and if we don't export owner_id
> > at uAPI inside the properties structure.
>
> I don't understand this remark. The owner_id of a property needs to be exported,
> how else would the application know to which object the property belongs?
I mean: there are two ways of exporting it:
1) entities, pads, links, interfaces, properties, ... would have a
properties_id field (where 0 would mean no properties associated).
That would allow a single property to be associated to multiple
elements (N:1). That would work really fine for "group" properties.
2) properties will have a owner_id. Mapping will be 1:1.
> >> + u32 type;
> >> + u32 payload_size;
> >> + struct list_head props;
> >> + char name[32];
> >> + u8 payload[];
> >> };
> >>
> >> /**
> >> @@ -266,6 +297,7 @@ enum media_entity_type {
> >> * @flags: Entity flags, as defined in
> >> * :ref:`include/uapi/linux/media.h <media_header>`
> >> * (seek for ``MEDIA_ENT_FL_*``)
> >> + * @inited: Non-zero if this struct was initialized.
> >> * @num_pads: Number of sink and source pads.
> >> * @num_links: Total number of links, forward and back, enabled and disabled.
> >> * @num_backlinks: Number of backlinks
> >> @@ -273,6 +305,7 @@ enum media_entity_type {
> >> * re-used if entities are unregistered or registered again.
> >> * @pads: Pads array with the size defined by @num_pads.
> >> * @links: List of data links.
> >> + * @props: List of entity properties.
> >> * @ops: Entity operations.
> >> * @stream_count: Stream count for the entity.
> >> * @use_count: Use count for the entity.
> >> @@ -300,6 +333,7 @@ struct media_entity {
> >> enum media_entity_type obj_type;
> >> u32 function;
> >> unsigned long flags;
> >> + unsigned int inited;
> >>
> >> u16 num_pads;
> >> u16 num_links;
> >> @@ -308,6 +342,7 @@ struct media_entity {
> >>
> >> struct media_pad *pads;
> >> struct list_head links;
> >> + struct list_head props;
> >>
> >> const struct media_entity_operations *ops;
> >>
> >> @@ -362,6 +397,20 @@ struct media_intf_devnode {
> >> u32 minor;
> >> };
> >>
> >> +/**
> >> + * media_entity_init() - initialize the media entity struct
> >> + *
> >> + * @entity: pointer to &media_entity
> >> + */
> >> +static inline void media_entity_init(struct media_entity *entity)
> >> +{
> >> + INIT_LIST_HEAD(&entity->links);
> >> + INIT_LIST_HEAD(&entity->props);
> >> + entity->num_links = 0;
> >> + entity->num_backlinks = 0;
> >> + entity->inited = true;
> >> +}
> >> +
> >
> > As I said before, I would keep it together with the C file, as it makes
> > easier to read (except, of course, if are there any reason why you
> > would need to call it on a different C file).
>
> It needs to be called from both media-entity.c and media-device.c.
OK.
> But it might be better to just put it in media-entity.c as a regular function.
My personal preference would be to declare it as a regular function,
instead of inlining it every time it is needed (but I can live with
the way it is right now).
>
> >
> >> /**
> >> * media_entity_id() - return the media entity graph object id
> >> *
> >> @@ -595,6 +644,15 @@ static inline bool media_entity_enum_intersects(
> >> #define gobj_to_intf(gobj) \
> >> container_of(gobj, struct media_interface, graph_obj)
> >>
> >> +/**
> >> + * gobj_to_prop - returns the struct &media_prop pointer from the
> >> + * @gobj contained on it.
> >> + *
> >> + * @gobj: Pointer to the struct &media_gobj graph object
> >> + */
> >> +#define gobj_to_prop(gobj) \
> >> + container_of(gobj, struct media_prop, graph_obj)
> >> +
> >> /**
> >> * intf_to_devnode - returns the struct media_intf_devnode pointer from the
> >> * @intf contained on it.
> >
> >
> >
> > Thanks,
> > Mauro
> >
>
> Regards,
>
> Hans
Thanks,
Mauro
