Re: [PATCH 14/18] [media] media-device: export the entity function via new ioctl
Em Mon, 23 Nov 2015 19:46:22 +0200 Laurent Pinchartescreveu: > Hi Mauro, > > Thank you for the patch. > > On Sunday 06 September 2015 14:30:57 Mauro Carvalho Chehab wrote: > > Now that entities have a main function, expose it via > > MEDIA_IOC_G_TOPOLOGY ioctl. > > > > Signed-off-by: Mauro Carvalho Chehab > > > > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c > > index ccef9621d147..32090030c342 100644 > > --- a/drivers/media/media-device.c > > +++ b/drivers/media/media-device.c > > @@ -263,6 +263,7 @@ static long __media_device_get_topology(struct > > media_device *mdev, /* Copy fields to userspace struct if not error */ > > memset(, 0, sizeof(uentity)); > > uentity.id = entity->graph_obj.id; > > + uentity.function = entity->function; > > strncpy(uentity.name, entity->name, > > sizeof(uentity.name)); > > > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > > index 69433405aec2..d232cc680c67 100644 > > --- a/include/uapi/linux/media.h > > +++ b/include/uapi/linux/media.h > > @@ -284,7 +284,8 @@ struct media_links_enum { > > struct media_v2_entity { > > __u32 id; > > char name[64]; /* FIXME: move to a property? (RFC says so) */ > > - __u16 reserved[14]; > > + __u32 function; /* Main function of the entity */ > > Shouldn't we use kerneldoc instead of inline comments ? We don't use kernel-doc for uAPI. Instead, we document those via the media infrastructure DocBook. I don't object to use the same format here, but adding the uAPI stuff to both device-drivers.xml and media_api.xml doesn't seem right. That's said, we may eventually change the media infrastructure DocBook to also run the kernel-doc script and benefit of kernel-doc markups also for the uAPI, but this is out of the scope of this work, and would take some time to do it. So, at least for now, I would keep the comments like the above. > > Also, as this is the main function only, I'd mention that in the subject > line. OK. > The implementation itself looks fine to me, I'll discuss the API over the > documentation patch. Ok. > > > + __u16 reserved[12]; > > }; > > > > /* Should match the specific fields at media_intf_devnode */ > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/18] [media] media-device: export the entity function via new ioctl
Hi Mauro, Thank you for the patch. On Sunday 06 September 2015 14:30:57 Mauro Carvalho Chehab wrote: > Now that entities have a main function, expose it via > MEDIA_IOC_G_TOPOLOGY ioctl. > > Signed-off-by: Mauro Carvalho Chehab> > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c > index ccef9621d147..32090030c342 100644 > --- a/drivers/media/media-device.c > +++ b/drivers/media/media-device.c > @@ -263,6 +263,7 @@ static long __media_device_get_topology(struct > media_device *mdev, /* Copy fields to userspace struct if not error */ > memset(, 0, sizeof(uentity)); > uentity.id = entity->graph_obj.id; > + uentity.function = entity->function; > strncpy(uentity.name, entity->name, > sizeof(uentity.name)); > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > index 69433405aec2..d232cc680c67 100644 > --- a/include/uapi/linux/media.h > +++ b/include/uapi/linux/media.h > @@ -284,7 +284,8 @@ struct media_links_enum { > struct media_v2_entity { > __u32 id; > char name[64]; /* FIXME: move to a property? (RFC says so) */ > - __u16 reserved[14]; > + __u32 function; /* Main function of the entity */ Shouldn't we use kerneldoc instead of inline comments ? Also, as this is the main function only, I'd mention that in the subject line. The implementation itself looks fine to me, I'll discuss the API over the documentation patch. > + __u16 reserved[12]; > }; > > /* Should match the specific fields at media_intf_devnode */ -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/18] [media] media-device: export the entity function via new ioctl
On 09/06/2015 07:30 PM, Mauro Carvalho Chehab wrote: > Now that entities have a main function, expose it via > MEDIA_IOC_G_TOPOLOGY ioctl. > > Signed-off-by: Mauro Carvalho ChehabAcked-by: Hans Verkuil > > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c > index ccef9621d147..32090030c342 100644 > --- a/drivers/media/media-device.c > +++ b/drivers/media/media-device.c > @@ -263,6 +263,7 @@ static long __media_device_get_topology(struct > media_device *mdev, > /* Copy fields to userspace struct if not error */ > memset(, 0, sizeof(uentity)); > uentity.id = entity->graph_obj.id; > + uentity.function = entity->function; > strncpy(uentity.name, entity->name, > sizeof(uentity.name)); > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > index 69433405aec2..d232cc680c67 100644 > --- a/include/uapi/linux/media.h > +++ b/include/uapi/linux/media.h > @@ -284,7 +284,8 @@ struct media_links_enum { > struct media_v2_entity { > __u32 id; > char name[64]; /* FIXME: move to a property? (RFC says so) */ > - __u16 reserved[14]; > + __u32 function; /* Main function of the entity */ > + __u16 reserved[12]; > }; > > /* Should match the specific fields at media_intf_devnode */ > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/18] [media] media-device: export the entity function via new ioctl
Now that entities have a main function, expose it via MEDIA_IOC_G_TOPOLOGY ioctl. Signed-off-by: Mauro Carvalho Chehabdiff --git a/drivers/media/media-device.c b/drivers/media/media-device.c index ccef9621d147..32090030c342 100644 --- a/drivers/media/media-device.c +++ b/drivers/media/media-device.c @@ -263,6 +263,7 @@ static long __media_device_get_topology(struct media_device *mdev, /* Copy fields to userspace struct if not error */ memset(, 0, sizeof(uentity)); uentity.id = entity->graph_obj.id; + uentity.function = entity->function; strncpy(uentity.name, entity->name, sizeof(uentity.name)); diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h index 69433405aec2..d232cc680c67 100644 --- a/include/uapi/linux/media.h +++ b/include/uapi/linux/media.h @@ -284,7 +284,8 @@ struct media_links_enum { struct media_v2_entity { __u32 id; char name[64]; /* FIXME: move to a property? (RFC says so) */ - __u16 reserved[14]; + __u32 function; /* Main function of the entity */ + __u16 reserved[12]; }; /* Should match the specific fields at media_intf_devnode */ -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html